From 6597f117c18c966a60353d2129a76a3541a193a6 Mon Sep 17 00:00:00 2001 From: Tam Nguyen Duc <1218621+tamnd@users.noreply.github.com> Date: Sat, 22 Aug 2026 16:26:51 +0700 Subject: [PATCH] Say what the caller did wrong, and prove it in a suite Thirty four deliberately wrong programs, each one asserting the class it raises and the sentence it says, run by both providers. The bar is the one dx/15 section 4 sets: no crash, no leak, and a message that names what the caller did rather than naming a C function. Six of them did not clear that bar, so this fixes those too. A column read as the wrong type said "zu_result_col_i64 answered MISUSE"; it now names the column, what it holds and what the accessor reads. An appender whose connection closed under it said "zu_appender_flush answered MISUSE_CLOSED"; it now says the connection is closed. A result read after it closed handed back a Row that failed later on the first cell instead of refusing at the call. A file that is not a database was an internal error, which asks the caller to report a bug about a path they mistyped, and is now a connection error beside the file that is not there. And the entry points take a null path or a null statement with the parameter named rather than passing it on to a call that will trip over it somewhere else. The second half is lifecycle. Five hundred failed opens, a thousand connections, a thousand failed statements, each with the count of open file descriptors taken either side of them, because a client that leaks a handle per failure passes every single wrong program above and falls over in a server. There is also a test of the programs that look like misuse and are not, so that none of this turns into a client that refuses ordinary work. A MATCH on a label that does not exist is deliberately not in the table. Today it answers no rows rather than saying so, which is an engine matter rather than a client one, and asserting the message we want here would have meant asserting a behaviour that does not exist. --- .../main/java/dev/zudb/tck/MisuseTest.java | 610 ++++++++++++++++++ zudb/src/main/java/dev/zudb/Appender.java | 14 +- zudb/src/main/java/dev/zudb/Connection.java | 12 +- zudb/src/main/java/dev/zudb/Database.java | 10 +- zudb/src/main/java/dev/zudb/Diagnostic.java | 6 + zudb/src/main/java/dev/zudb/Loader.java | 2 + zudb/src/main/java/dev/zudb/Result.java | 52 ++ .../java/dev/zudb/ZuConnectionException.java | 7 + .../test/java/dev/zudb/DiagnosticTest.java | 6 +- 9 files changed, 712 insertions(+), 7 deletions(-) create mode 100644 zudb-tck/src/main/java/dev/zudb/tck/MisuseTest.java diff --git a/zudb-tck/src/main/java/dev/zudb/tck/MisuseTest.java b/zudb-tck/src/main/java/dev/zudb/tck/MisuseTest.java new file mode 100644 index 0000000..a34853d --- /dev/null +++ b/zudb-tck/src/main/java/dev/zudb/tck/MisuseTest.java @@ -0,0 +1,610 @@ +package dev.zudb.tck; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assumptions.assumeTrue; + +import dev.zudb.Appender; +import dev.zudb.Config; +import dev.zudb.Connection; +import dev.zudb.Database; +import dev.zudb.Frame; +import dev.zudb.Loader; +import dev.zudb.Result; +import dev.zudb.Statement; +import dev.zudb.ZuClosedException; +import dev.zudb.ZuConnectionException; +import dev.zudb.ZuException; +import dev.zudb.ZuProgrammingException; +import dev.zudb.ZuSyntaxException; +import dev.zudb.ZuTransactionException; +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.LongBuffer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Stream; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.DynamicTest; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestFactory; +import org.junit.jupiter.api.function.ThrowingConsumer; +import org.junit.jupiter.api.io.TempDir; + +/** + * Programs that are wrong, and what each one is told. + * + *

Every case here is a mistake somebody makes: a path that is not a + * database, a handle used after it closed, a column read as the type it is + * not, a row with the wrong number of values in it. The bar for all of them is + * the same and it has three parts. The process does not die. Nothing is left + * open behind it. And the message names what the caller did, in their own + * terms, rather than naming a C function they have never heard of. + * + *

That third part is the reason this file asserts on message text at all. + * Asserting on the class alone would pass for a client that answers every + * mistake with the same sentence, and a sentence like {@code zu_result_col_i64 + * answered MISUSE} is exactly the sentence that passes. The text is matched as + * a substring, because most of these carry a temporary path through them, and + * the substring is chosen to be the part a reader would act on. + * + *

The second half of the file is the lifecycle half: hundreds of failures + * in a row, and a count of open file descriptors either side of them. A client + * that leaks a handle per failed open is a client that works in a test and + * falls over in a server, and no single wrong program catches that. + */ +public class MisuseTest { + + @TempDir Path dir; + + @BeforeAll + static void engine() { + Libzu.require(); + } + + /** One wrong program, what it should raise, and what it should say. */ + private record Wrong( + String what, Class raises, String says, ThrowingConsumer run) {} + + private static final List WRONG = + List.of( + // Paths, and files that are not databases. + new Wrong( + "opening a database that is not there", + ZuConnectionException.class, + "gone.zu1", + where -> Database.open(where.resolve("gone.zu1")).close()), + new Wrong( + "creating a database where one already is", + ZuException.class, + "twice.zu1", + where -> { + people(where.resolve("twice.zu1")); + Database.create(where.resolve("twice.zu1")).close(); + }), + new Wrong( + "opening a file too small to hold a header", + ZuConnectionException.class, + "too short to be a zu1 database", + where -> { + Path path = where.resolve("small.zu1"); + Files.writeString(path, "not a database at all", StandardCharsets.UTF_8); + Database.open(path).close(); + }), + new Wrong( + "opening a file that is not a database", + ZuConnectionException.class, + "bad magic, not a zu1 file", + where -> { + Path path = where.resolve("big.zu1"); + Files.write(path, new byte[4096]); + Database.open(path).close(); + }), + new Wrong( + "opening a database named by nothing at all", + NullPointerException.class, + "path", + where -> Database.open((Path) null).close()), + + // A connection that refuses writes. + new Wrong( + "writing through a read-only connection", + ZuException.class, + "which is open read-only", + where -> { + Path path = people(where.resolve("reader.zu1")); + try (Database db = Database.open(path, Config.defaults().withReadOnly(true)); + Connection conn = db.connect()) { + conn.execute("INSERT (:Person {id: 4, name: 'hedy'})"); + } + }), + new Wrong( + "an appender on a read-only connection", + ZuException.class, + "an appender writes and the connection is read-only", + where -> { + Path path = people(where.resolve("reader.zu1")); + try (Database db = Database.open(path, Config.defaults().withReadOnly(true)); + Connection conn = db.connect()) { + conn.appender("Person").close(); + } + }), + + // Statements. + new Wrong( + "a statement with a typo in it", + ZuSyntaxException.class, + "found 'RETRUN'", + where -> scratch(where, conn -> conn.execute("MATCH (p:Person) RETRUN p.id"))), + new Wrong( + "a parameter that was never bound", + ZuSyntaxException.class, + "missing parameter $id", + where -> + scratch( + where, + conn -> { + try (Statement s = + conn.prepare("MATCH (p:Person) WHERE p.id = $id RETURN p.name")) { + s.execute().close(); + } + })), + new Wrong( + // A MATCH on a label that does not exist is not here, because + // today it answers no rows rather than saying so. An appender + // is the call that does refuse, and it is the one a program + // gets wrong most often anyway. + "a table that is not there", + ZuException.class, + "no node table or rel table 'Nobody'", + where -> scratch(where, conn -> conn.appender("Nobody").close())), + new Wrong( + "a variable that was never defined", + ZuSyntaxException.class, + "variable 'nobody' is not defined", + where -> scratch(where, conn -> conn.execute("RETURN nobody"))), + new Wrong( + "a statement that is nothing at all", + NullPointerException.class, + "statement", + where -> scratch(where, conn -> conn.execute(null))), + new Wrong( + "a statement on a connection that was closed", + ZuClosedException.class, + "this connection is closed", + where -> { + Connection conn = Connection.create(where.resolve("shut.zu1")); + conn.close(); + conn.execute("RETURN 1"); + }), + + // Reading a result. + new Wrong( + "a row past the end of the result", + ZuProgrammingException.class, + "row 99 of a result with 3 of them", + where -> read(where, result -> result.row(99))), + new Wrong( + "a column the result does not have", + ZuProgrammingException.class, + "no column called nope", + where -> read(where, result -> result.row(0).getLong("nope"))), + new Wrong( + "a column of strings read as integers", + ZuProgrammingException.class, + "longs(1) reads a column of INT", + where -> read(where, result -> result.longs(1))), + new Wrong( + "a result read after it closed", + ZuClosedException.class, + "this result is closed", + where -> + scratch( + where, + conn -> { + Result result = conn.query("RETURN 1 AS one"); + result.close(); + result.row(0); + })), + + // Writing through an appender. + new Wrong( + "a value of a type the column does not hold", + ZuException.class, + "column 'id' of 'Person' holds integers", + where -> append(where, rows -> rows.append("four").append("hedy").endRow())), + new Wrong( + "a row with a value missing from it", + ZuException.class, + "and 'Person' takes 2: id, name", + where -> append(where, rows -> rows.append(4L).endRow().flush())), + new Wrong( + "a row with a value too many in it", + ZuException.class, + "already carries the 2 values 'Person' takes", + where -> append(where, rows -> rows.append(4L).append("hedy").append(5L))), + new Wrong( + "a null in a row", + ZuProgrammingException.class, + "there is no null to append", + where -> append(where, rows -> rows.row(4L, null))), + new Wrong( + "a value of a class no column holds", + ZuProgrammingException.class, + "no column holds one of those", + where -> append(where, rows -> rows.row(4L, List.of("hedy")))), + new Wrong( + "an appender used after it closed", + ZuClosedException.class, + "this appender is closed", + where -> + append( + where, + rows -> { + rows.close(); + rows.append(4L); + })), + new Wrong( + "an appender used after its connection closed", + ZuClosedException.class, + "the connection this appender writes through is closed", + where -> { + Path path = people(where.resolve("orphan.zu1")); + Database db = Database.open(path); + Connection conn = db.connect(); + Appender rows = conn.appender("Person"); + conn.close(); + db.close(); + try { + rows.append(4L).append("hedy").endRow().flush(); + } finally { + rows.close(); + } + }), + + // Building a database. + new Wrong( + "a column with the wrong number of values in it", + ZuException.class, + "against a table of 3 rows", + where -> { + try (Loader loader = Loader.create(where.resolve("short.zu1"))) { + loader.table("Person", "Knows", 3); + loader.column("id", 1L, 2L); + } + }), + new Wrong( + "an edge to a row that is not there", + ZuException.class, + "a table with 2 rows in it", + where -> { + try (Loader loader = Loader.create(where.resolve("edge.zu1"))) { + loader.table("Person", "Knows", 2); + loader.column("id", 1L, 2L); + loader.column("name", "ada", "grace"); + loader.edges(new int[] {0}, new int[] {9}); + } + }), + new Wrong( + "a column before there is a table for it to be in", + ZuException.class, + "the loader has no table yet", + where -> { + try (Loader loader = Loader.create(where.resolve("early.zu1"))) { + loader.column("id", 1L, 2L); + } + }), + new Wrong( + "the same table twice", + ZuException.class, + "this loader already has a table", + where -> { + try (Loader loader = Loader.create(where.resolve("again.zu1"))) { + loader.table("Person", "Knows", 2); + loader.table("Person", "Knows", 2); + } + }), + new Wrong( + "a loader used after it closed", + ZuClosedException.class, + "this loader is closed", + where -> { + Loader loader = Loader.create(where.resolve("shut.zu1")); + loader.table("Person", "Knows", 1); + loader.column("id", 1L); + loader.column("name", "ada"); + loader.finish(); + loader.close(); + loader.table("Other", "Knows", 1); + }), + + // Transactions. + new Wrong( + "committing when nothing is running", + ZuTransactionException.class, + "no transaction running on this session to commit", + where -> scratch(where, Connection::commit)), + new Wrong( + "rolling back when nothing is running", + ZuTransactionException.class, + "no transaction running on this session to roll back", + where -> scratch(where, Connection::rollback)), + new Wrong( + "a transaction inside a transaction", + ZuTransactionException.class, + "one does not start inside another", + where -> + scratch( + where, + conn -> { + conn.begin(); + conn.begin(); + })), + + // Frames. + new Wrong( + "a frame whose columns are on the heap", + ZuException.class, + "ByteBuffer.allocateDirect", + where -> { + try (Frame frame = Frame.of("Heap", 2)) { + frame.column("id", LongBuffer.allocate(2)); + } + }), + new Wrong( + "a frame registered inside a transaction", + ZuTransactionException.class, + "a frame is registered on the session", + where -> + scratch( + where, + conn -> { + try (Frame frame = Frame.of("Heap", 2)) { + frame.column("id", direct(1L, 2L)); + conn.begin(); + conn.register(frame); + } + }))); + + @Test + void thereAreWrongProgramsToRun() { + assertTrue(WRONG.size() >= 30, "the suite is meant to be a wide table, not a sample"); + } + + @TestFactory + List everyWrongProgramSaysWhatIsWrong() { + List tests = new ArrayList<>(); + for (int i = 0; i < WRONG.size(); i++) { + Wrong wrong = WRONG.get(i); + Path where = dir.resolve("case" + i); + tests.add( + DynamicTest.dynamicTest( + wrong.what(), + () -> { + Files.createDirectories(where); + Throwable thrown = + assertThrows( + wrong.raises(), + () -> wrong.run().accept(where), + wrong.what() + " raised nothing at all"); + String said = thrown.getMessage(); + assertNotNull(said, wrong.what() + " raised " + thrown.getClass() + " with no words"); + assertTrue( + said.contains(wrong.says()), + wrong.what() + + " said\n " + + said + + "\nwhich does not contain\n " + + wrong.says()); + })); + } + return tests; + } + + @Test + void fiveHundredFailedOpensLeaveNothingOpen() throws IOException { + Path gone = dir.resolve("never-existed.zu1"); + for (int i = 0; i < 20; i++) { + assertThrows(ZuException.class, () -> Database.open(gone).close()); + } + long before = openFiles(); + assumeTrue(before > 0, "no way to count open files here"); + for (int i = 0; i < 500; i++) { + assertThrows(ZuException.class, () -> Database.open(gone).close()); + } + long after = openFiles(); + assertTrue( + after - before <= 8, + "500 failed opens went from " + before + " open files to " + after); + } + + @Test + void aThousandConnectionsOpenedAndClosedLeaveNothingBehind() throws IOException { + Path path = people(dir.resolve("thousand.zu1")); + try (Database db = Database.open(path)) { + for (int i = 0; i < 20; i++) { + db.connect().close(); + } + long before = openFiles(); + assumeTrue(before > 0, "no way to count open files here"); + for (int i = 0; i < 1000; i++) { + try (Connection conn = db.connect(); + Result result = conn.query("MATCH (p:Person) RETURN p.id")) { + assertEquals(3L, result.rows()); + } + } + long after = openFiles(); + assertTrue( + after - before <= 8, + "1000 connections went from " + before + " open files to " + after); + } + } + + @Test + void aConnectionClosedWithThingsOpenOnItDoesNotTakeTheProcessWithIt() { + Path path = people(dir.resolve("early-close.zu1")); + try (Database db = Database.open(path)) { + for (int i = 0; i < 100; i++) { + Connection conn = db.connect(); + Result result = conn.query("MATCH (p:Person) RETURN p.id, p.name"); + Appender rows = conn.appender("Person"); + conn.close(); + // Whatever these two answer now, they answer it rather than taking + // the process down, and closing them afterwards is still allowed. + result.close(); + rows.close(); + } + } + } + + @Test + void aStatementThatFailedWroteNothingAndLeftTheConnectionAlone() { + Path path = people(dir.resolve("failed.zu1")); + try (Database db = Database.open(path); + Connection conn = db.connect()) { + assertThrows(ZuException.class, () -> conn.execute("MATCH (p:Person) RETRUN p.id")); + assertThrows(ZuException.class, () -> conn.execute("RETURN nobody")); + try (Result result = conn.query("MATCH (p:Person) RETURN p.id")) { + assertEquals(3L, result.rows()); + } + } + } + + @Test + void aThousandFailedStatementsLeaveNothingBehind() throws IOException { + Path path = people(dir.resolve("failures.zu1")); + try (Database db = Database.open(path); + Connection conn = db.connect()) { + for (int i = 0; i < 20; i++) { + assertThrows(ZuException.class, () -> conn.execute("MATCH (p:Person) RETRUN p.id")); + } + long before = openFiles(); + assumeTrue(before > 0, "no way to count open files here"); + for (int i = 0; i < 1000; i++) { + assertThrows(ZuException.class, () -> conn.execute("MATCH (p:Person) RETRUN p.id")); + } + long after = openFiles(); + assertTrue( + after - before <= 8, + "1000 failed statements went from " + before + " open files to " + after); + try (Result result = conn.query("MATCH (p:Person) RETURN p.id")) { + assertEquals(3L, result.rows()); + } + } + } + + @Test + void theProgramsThatLookLikeMisuseAndAreNot() { + Path path = people(dir.resolve("fine.zu1")); + try (Database db = Database.open(path); + Connection conn = db.connect()) { + // Closing twice is not a mistake, and neither is closing something that + // was already finished. + Result result = conn.query("MATCH (p:Person) RETURN p.id, p.name"); + result.close(); + result.close(); + assertTrue(result.isClosed()); + + try (Appender rows = conn.appender("Person")) { + assertEquals(0L, rows.finish()); + assertTrue(rows.isFinished()); + } + + // A result with no rows in it is an answer, not a failure, and reading + // its columns as a borrowed lane is allowed even though there is + // nothing in the lane. + try (Result none = conn.query("MATCH (p:Person) WHERE p.id = 99 RETURN p.id")) { + assertEquals(0L, none.rows()); + assertEquals(0, none.longs(0).remaining()); + assertEquals(0L, none.stream().count()); + } + + // A transaction that did nothing still rolls back. + conn.begin(); + conn.rollback(); + + // And the connection is still the connection afterwards. + try (Result again = conn.query("MATCH (p:Person) RETURN p.id")) { + assertEquals(3L, again.rows()); + } + assertFalse(conn.isClosed()); + } + } + + /** A two column Person table with three rows in it, at the path given. */ + private static Path people(Path path) { + try (Loader loader = Loader.create(path)) { + loader.table("Person", "Knows", 3); + loader.column("id", 1L, 2L, 3L); + loader.column("name", "ada", "grace", "lynn"); + loader.finish(); + } + return path; + } + + /** Runs a wrong program against a fresh database of three people. */ + private static void scratch(Path where, ThrowingConsumer run) throws Throwable { + Path path = people(where.resolve("scratch.zu1")); + try (Database db = Database.open(path); + Connection conn = db.connect()) { + run.accept(conn); + } + } + + /** Runs a wrong program against a result of three people, id then name. */ + private static void read(Path where, ThrowingConsumer run) throws Throwable { + scratch( + where, + conn -> { + try (Result result = conn.query("MATCH (p:Person) RETURN p.id AS id, p.name AS name")) { + run.accept(result); + } + }); + } + + /** Runs a wrong program against an appender on a table of three people. */ + private static void append(Path where, ThrowingConsumer run) throws Throwable { + scratch( + where, + conn -> { + try (Appender rows = conn.appender("Person")) { + run.accept(rows); + } + }); + } + + /** A direct buffer of longs, which is the only kind a frame will take. */ + private static LongBuffer direct(long... values) { + LongBuffer buffer = + ByteBuffer.allocateDirect(values.length * Long.BYTES) + .order(java.nio.ByteOrder.nativeOrder()) + .asLongBuffer(); + buffer.put(values).flip(); + return buffer; + } + + /** + * How many files this process has open, or -1 where there is no way to ask. + * + *

Both platforms this runs on keep a directory of the process's own + * descriptors, which is cheaper to read than a management bean and does not + * put a module on the path for one number. + */ + private static long openFiles() throws IOException { + for (String each : new String[] {"/proc/self/fd", "/dev/fd"}) { + Path fds = Path.of(each); + if (Files.isDirectory(fds)) { + try (Stream open = Files.list(fds)) { + return open.count(); + } + } + } + return -1; + } +} diff --git a/zudb/src/main/java/dev/zudb/Appender.java b/zudb/src/main/java/dev/zudb/Appender.java index 5bdcf4a..c7f9798 100644 --- a/zudb/src/main/java/dev/zudb/Appender.java +++ b/zudb/src/main/java/dev/zudb/Appender.java @@ -51,11 +51,13 @@ public final class Appender implements AutoCloseable { private final ZuBinding zu; private final AtomicLong handle; + private final Connection conn; private long finished = -1; - Appender(ZuBinding zu, long handle) { + Appender(ZuBinding zu, long handle, Connection conn) { this.zu = zu; this.handle = new AtomicLong(handle); + this.conn = conn; } /** @@ -401,6 +403,16 @@ private long open() { throw new ZuClosedException( Diagnostic.misuse(Status.MISUSE_CLOSED, "this appender is closed")); } + // The engine refuses this too, and refuses it without an error record + // attached, so what a caller would otherwise be told is the name of a C + // function. An appender outliving its connection is an ordinary mistake + // in a program that closes things in the wrong order, and the sentence + // that names it is worth more than the one that names us. + if (conn != null && conn.isClosed()) { + throw new ZuClosedException( + Diagnostic.misuse( + Status.MISUSE_CLOSED, "the connection this appender writes through is closed")); + } return h; } } diff --git a/zudb/src/main/java/dev/zudb/Connection.java b/zudb/src/main/java/dev/zudb/Connection.java index 312c896..3e38753 100644 --- a/zudb/src/main/java/dev/zudb/Connection.java +++ b/zudb/src/main/java/dev/zudb/Connection.java @@ -5,6 +5,7 @@ import java.time.Duration; import java.util.ArrayList; import java.util.List; +import java.util.Objects; import java.util.concurrent.atomic.AtomicLong; import java.util.function.Supplier; @@ -56,6 +57,7 @@ public final class Connection implements AutoCloseable { * @return the connection, which the caller closes */ public static Connection open(Path path) { + Objects.requireNonNull(path, "path"); ZuBinding zu = Zu.binding(); return new Connection(zu, zu.open(path.toString())); } @@ -67,7 +69,7 @@ public static Connection open(Path path) { * @return the connection, which the caller closes */ public static Connection open(String path) { - return open(Path.of(path)); + return open(Path.of(Objects.requireNonNull(path, "path"))); } /** @@ -78,6 +80,7 @@ public static Connection open(String path) { * @return the connection, which the caller closes */ public static Connection create(Path path) { + Objects.requireNonNull(path, "path"); ZuBinding zu = Zu.binding(); return new Connection(zu, zu.create(path.toString())); } @@ -89,7 +92,7 @@ public static Connection create(Path path) { * @return the connection, which the caller closes */ public static Connection create(String path) { - return create(Path.of(path)); + return create(Path.of(Objects.requireNonNull(path, "path"))); } /** @@ -120,6 +123,7 @@ public static Connection memory() { * @return the result, which the caller closes */ public Result query(String statement) { + Objects.requireNonNull(statement, "statement"); return new Result(zu, zu.query(open(), statement), this); } @@ -144,6 +148,7 @@ public void execute(String statement) { * @return the statement, which the caller closes */ public Statement prepare(String statement) { + Objects.requireNonNull(statement, "statement"); return new Statement(zu, zu.prepare(open(), statement), this); } @@ -158,7 +163,8 @@ public Statement prepare(String statement) { * @return the appender, which the caller closes */ public Appender appender(String table) { - return new Appender(zu, zu.appenderOpen(open(), table)); + Objects.requireNonNull(table, "table"); + return new Appender(zu, zu.appenderOpen(open(), table), this); } /** diff --git a/zudb/src/main/java/dev/zudb/Database.java b/zudb/src/main/java/dev/zudb/Database.java index d2324ca..ab9f6d1 100644 --- a/zudb/src/main/java/dev/zudb/Database.java +++ b/zudb/src/main/java/dev/zudb/Database.java @@ -2,6 +2,7 @@ import dev.zudb.spi.ZuBinding; import java.nio.file.Path; +import java.util.Objects; import java.util.concurrent.atomic.AtomicLong; /** @@ -53,6 +54,8 @@ public static Database open(Path path) { * @return the database */ public static Database open(Path path, Config config) { + Objects.requireNonNull(path, "path"); + Objects.requireNonNull(config, "config"); ZuBinding zu = Zu.binding(); return new Database( zu, @@ -68,7 +71,7 @@ public static Database open(Path path, Config config) { * @return the database */ public static Database open(String path) { - return open(Path.of(path), Config.defaults()); + return open(Path.of(Objects.requireNonNull(path, "path")), Config.defaults()); } /** @@ -79,7 +82,7 @@ public static Database open(String path) { * @return the database */ public static Database open(String path, Config config) { - return open(Path.of(path), config); + return open(Path.of(Objects.requireNonNull(path, "path")), config); } /** @@ -105,6 +108,8 @@ public static Database create(Path path) { * @return the database */ public static Database create(Path path, Config config) { + Objects.requireNonNull(path, "path"); + Objects.requireNonNull(config, "config"); ZuBinding zu = Zu.binding(); return new Database( zu, @@ -133,6 +138,7 @@ public static Database memory() { * @return the database */ public static Database memory(Config config) { + Objects.requireNonNull(config, "config"); ZuBinding zu = Zu.binding(); return new Database( zu, zu.databaseMemory(config.memoryLimit(), config.threads(), config.readOnly())); diff --git a/zudb/src/main/java/dev/zudb/Diagnostic.java b/zudb/src/main/java/dev/zudb/Diagnostic.java index 5f5be19..33057ba 100644 --- a/zudb/src/main/java/dev/zudb/Diagnostic.java +++ b/zudb/src/main/java/dev/zudb/Diagnostic.java @@ -80,6 +80,12 @@ public ZuException toException() { case CONFLICT: return new ZuTransactionException(this); case IO: + case CORRUPT: + // A file that is not a database is the same mistake as a file that + // is not there: a path that does not lead to a database. Almost + // every one of these is a caller who mistyped a path or pointed at + // the wrong file, and calling that an internal error tells them to + // file a bug about somebody else's code. return new ZuConnectionException(this); default: return new ZuInternalException(this); diff --git a/zudb/src/main/java/dev/zudb/Loader.java b/zudb/src/main/java/dev/zudb/Loader.java index aa48d07..16ed984 100644 --- a/zudb/src/main/java/dev/zudb/Loader.java +++ b/zudb/src/main/java/dev/zudb/Loader.java @@ -7,6 +7,7 @@ import java.nio.file.Path; import java.util.Arrays; import java.util.List; +import java.util.Objects; import java.util.concurrent.atomic.AtomicLong; /** @@ -63,6 +64,7 @@ private Loader(ZuBinding zu, long handle) { * @throws ZuException if the path exists or cannot be written */ public static Loader create(Path path) { + Objects.requireNonNull(path, "path"); ZuBinding zu = Zu.binding(); return new Loader(zu, zu.loaderCreate(path.toString())); } diff --git a/zudb/src/main/java/dev/zudb/Result.java b/zudb/src/main/java/dev/zudb/Result.java index 9660cfa..ad221d1 100644 --- a/zudb/src/main/java/dev/zudb/Result.java +++ b/zudb/src/main/java/dev/zudb/Result.java @@ -141,6 +141,11 @@ public int columnIndex(String name) { * @return the row, which is good until this result closes */ public Row row(long index) { + // A closed result has no rows, so this refuses here rather than handing + // back a Row that fails on the first cell read. The wrong call is this + // one, and a failure two lines further on is a failure a caller has to + // work backwards from. + open(); if (index < 0 || index >= rows) { throw new ZuProgrammingException( Diagnostic.misuse( @@ -263,6 +268,7 @@ public List notices() { */ public LongBuffer longs(int column) { checkColumn(column); + checkHolds("longs", column, Type.INT, Type.BOOL); LongBuffer b = zu.colLongs(open(), column, rows); return b == null ? LongBuffer.allocate(0).asReadOnlyBuffer() : b; } @@ -275,6 +281,7 @@ public LongBuffer longs(int column) { */ public DoubleBuffer doubles(int column) { checkColumn(column); + checkHolds("doubles", column, Type.FLOAT, Type.INT); DoubleBuffer b = zu.colDoubles(open(), column, rows); return b == null ? DoubleBuffer.allocate(0).asReadOnlyBuffer() : b; } @@ -292,6 +299,7 @@ public DoubleBuffer doubles(int column) { */ public LongBuffer nodeOffsets(int column) { checkColumn(column); + checkHolds("nodeOffsets", column, Type.NODE); LongBuffer b = zu.colNodeOffsets(open(), column, rows); return b == null ? LongBuffer.allocate(0).asReadOnlyBuffer() : b; } @@ -464,6 +472,50 @@ long open() { return h; } + /** + * Refuses a borrowed column the accessor cannot read, in this client's own + * words. + * + *

The engine refuses it too, but a call that comes back without a + * {@code zu_error} on it leaves nothing to say except which C function was + * called, and "zu_result_col_i64 answered MISUSE" is a sentence about our + * implementation rather than about the caller's program. Here there is a + * column name and a type to name, so this says them. + * + *

The first row is what is read, because a borrowed column is one lane + * of one type and the first cell is the cheapest place that says which. A + * null there says nothing, so it passes and the engine has the last word. + */ + private void checkHolds(String accessor, int column, Type... reads) { + if (rows == 0) { + return; + } + Type holds = cellType(0, column); + if (holds == Type.NULL) { + return; + } + for (Type ok : reads) { + if (holds == ok) { + return; + } + } + throw new ZuProgrammingException( + Diagnostic.misuse( + Status.MISUSE, + "column " + + column + + " of this result is " + + names.get(column) + + ", which holds " + + holds + + ", and " + + accessor + + "(" + + column + + ") reads a column of " + + reads[0])); + } + void checkColumn(int column) { if (column < 0 || column >= columns) { throw new ZuProgrammingException( diff --git a/zudb/src/main/java/dev/zudb/ZuConnectionException.java b/zudb/src/main/java/dev/zudb/ZuConnectionException.java index 650f55a..2c2ffb7 100644 --- a/zudb/src/main/java/dev/zudb/ZuConnectionException.java +++ b/zudb/src/main/java/dev/zudb/ZuConnectionException.java @@ -7,6 +7,13 @@ *

Nothing here is about the statement. A path that is not a zu database, a * file the process may not open, a disk that answered an error: the text was * never the problem and rewriting it will not help. + * + *

A file whose contents are not a database is here rather than in + * {@link ZuInternalException}, and the two readings of that were weighed. A + * header that says something impossible could be a database this process + * corrupted, and it could be a JPEG somebody pointed at. The second is what + * almost every one of these is, so this is the class that sends a caller to + * look at the path they passed rather than to open a bug. */ public class ZuConnectionException extends ZuException { diff --git a/zudb/src/test/java/dev/zudb/DiagnosticTest.java b/zudb/src/test/java/dev/zudb/DiagnosticTest.java index 5a9465a..c117390 100644 --- a/zudb/src/test/java/dev/zudb/DiagnosticTest.java +++ b/zudb/src/test/java/dev/zudb/DiagnosticTest.java @@ -49,7 +49,11 @@ void aStatusWithNoCodePicksTheException() { ZuInterruptedException.class, diagnostic(Status.INTERRUPTED, null).toException()); assertInstanceOf(ZuTransactionException.class, diagnostic(Status.CONFLICT, null).toException()); assertInstanceOf(ZuConnectionException.class, diagnostic(Status.IO, null).toException()); - assertInstanceOf(ZuInternalException.class, diagnostic(Status.CORRUPT, null).toException()); + // A file that is not a database is a path mistake, not a bug in the + // engine, so it lands beside the file that is not there rather than in + // the class that asks the caller to report it. + assertInstanceOf(ZuConnectionException.class, diagnostic(Status.CORRUPT, null).toException()); + assertInstanceOf(ZuInternalException.class, diagnostic(Status.UNKNOWN, null).toException()); } @Test