diff --git a/Content.Tests/DMProject/Tests/Database/blob.dm b/Content.Tests/DMProject/Tests/Database/blob.dm new file mode 100644 index 0000000000..093d72820b --- /dev/null +++ b/Content.Tests/DMProject/Tests/Database/blob.dm @@ -0,0 +1,27 @@ +/proc/RunTest() + var/database/db = new("blob.db") + var/database/query/query = new("CREATE TABLE test (value)") + query.Execute(db) + + var/icon/icon = new + query.Add("INSERT INTO test VALUES (?)", icon) + query.Execute(db) + ASSERT(query.RowsAffected() == 1) + + query.Add("SELECT typeof(value) FROM test") + query.Execute(db) + ASSERT(query.NextRow()) + var/list/type_row = query.GetRowData() + ASSERT(type_row["typeof(value)"] == "blob") + ASSERT(!query.Error()) + + // BYOND stores icons as BLOBs but returns null for BLOB columns through its DM API. + query.Add("SELECT value FROM test") + query.Execute(db) + ASSERT(query.NextRow()) + ASSERT(isnull(query.GetColumn(0))) + ASSERT(!query.Error()) + + del(query) + del(db) + fdel("blob.db") diff --git a/Content.Tests/DMProject/Tests/Database/clear.dm b/Content.Tests/DMProject/Tests/Database/clear.dm index 5790d9d641..e414e56de4 100644 --- a/Content.Tests/DMProject/Tests/Database/clear.dm +++ b/Content.Tests/DMProject/Tests/Database/clear.dm @@ -7,8 +7,8 @@ // Add with no parameters does nothing query.Add() - // Execute without a command does nothing - query.Execute() + // Execute without a command does nothing and returns false. + ASSERT(!query.Execute(db)) // and shouldn't report an error ASSERT(!query.Error()) diff --git a/Content.Tests/DMProject/Tests/Database/read.dm b/Content.Tests/DMProject/Tests/Database/read.dm index 8da6dece5a..04a1995cc2 100644 --- a/Content.Tests/DMProject/Tests/Database/read.dm +++ b/Content.Tests/DMProject/Tests/Database/read.dm @@ -1,4 +1,5 @@ /proc/RunTest() + fdel("database.db") var/database/db = new("database.db") var/database/query/query = new("CREATE TABLE test (id int, name string, points float)") @@ -20,22 +21,23 @@ ASSERT(assoc["name"] == "foo") ASSERT(assoc["points"] == 1.5) - ASSERT(query.GetColumn(0) == 1) - ASSERT(query.GetColumn(1) == "foo") - ASSERT(query.GetColumn(2) == 1.5) + ASSERT(isnull(query.GetColumn(0))) + ASSERT(query.GetColumn(1) == 1) + ASSERT(query.GetColumn(2) == "foo") + ASSERT(query.GetColumn(3) == 1.5) var/list/columns = query.Columns() ASSERT(columns[1] == "id") ASSERT(columns[2] == "name") ASSERT(columns[3] == "points") - ASSERT(query.Columns(0) == "id") - ASSERT(query.Columns(1) == "name") - ASSERT(query.Columns(2) == "points") + ASSERT(query.Columns(0) == "") + ASSERT(query.Columns(1) == "id") + ASSERT(query.Columns(2) == "name") + ASSERT(query.Columns(3) == "points") ASSERT(!query.Columns(10)) - - ASSERT(query.Error() && query.ErrorMsg()) + ASSERT(!query.Error()) query.Close() db.Close() @@ -46,10 +48,10 @@ query.Execute(db) query.NextRow() - ASSERT(query.GetColumn(0) == 1) + ASSERT(query.GetColumn(1) == 1) ASSERT(!query.GetColumn(10)) - ASSERT(query.Error() && query.ErrorMsg()) + ASSERT(!query.Error()) del(query) del(db) diff --git a/Content.Tests/DMProject/Tests/Database/reexecute.dm b/Content.Tests/DMProject/Tests/Database/reexecute.dm new file mode 100644 index 0000000000..1fe7587534 --- /dev/null +++ b/Content.Tests/DMProject/Tests/Database/reexecute.dm @@ -0,0 +1,25 @@ +/proc/RunTest() + fdel("reexecute.db") + var/database/db = new("reexecute.db") + var/database/query/query = new("CREATE TABLE test (value)") + ASSERT(query.Execute(db)) + + query.Add("INSERT INTO test VALUES (1), (2)") + ASSERT(query.Execute(db)) + + query.Add("SELECT value FROM test ORDER BY value") + ASSERT(query.Execute(db)) + ASSERT(query.NextRow()) + var/list/first_row = query.GetRowData() + ASSERT(first_row["value"] == 1) + + // Re-executing must discard the still-open reader and restart the result set. + ASSERT(query.Execute(db)) + ASSERT(query.NextRow()) + var/list/reexecuted_first_row = query.GetRowData() + ASSERT(reexecuted_first_row["value"] == 1) + ASSERT(!query.Error()) + + del(query) + del(db) + fdel("reexecute.db") diff --git a/OpenDreamRuntime/Objects/Types/DreamObjectDatabaseQuery.cs b/OpenDreamRuntime/Objects/Types/DreamObjectDatabaseQuery.cs index 3d6fb1954c..551765797e 100644 --- a/OpenDreamRuntime/Objects/Types/DreamObjectDatabaseQuery.cs +++ b/OpenDreamRuntime/Objects/Types/DreamObjectDatabaseQuery.cs @@ -7,6 +7,7 @@ namespace OpenDreamRuntime.Objects.Types; public sealed class DreamObjectDatabaseQuery(DreamObjectDefinition objectDefinition) : DreamObject(objectDefinition) { private SqliteCommand? _command; private SqliteDataReader? _reader; + private DreamObjectDatabase? _temporaryDatabase; private string? _errorMessage; private int? _errorCode; @@ -14,19 +15,41 @@ public sealed class DreamObjectDatabaseQuery(DreamObjectDefinition objectDefinit public override void Initialize(DreamProcArguments args) { base.Initialize(args); - if (!args.GetArgument(0).TryGetValueAsString(out var command)) { + var commandArgument = args.GetArgument(0); + if (commandArgument.IsNull) { return; } + if (!commandArgument.TryGetValueAsString(out var command)) { + throw new DMCrashRuntime("Invalid database query text"); + } + SetupCommand(command, args.Values[1..]); } protected override void HandleDeletion() { ClearCommand(); CloseReader(); + CloseTemporaryDatabase(); base.HandleDeletion(); } + /// + /// Associates a database with this query that was opened from a filename + /// (BYOND's query.Execute(filename) overload). The query owns it for + /// the duration of reading rows, and closes it when done. + /// + public void SetTemporaryDatabase(DreamObjectDatabase database) { + CloseTemporaryDatabase(); + _temporaryDatabase = database; + } + + private void CloseTemporaryDatabase() { + _temporaryDatabase?.Close(); + _temporaryDatabase?.DecRef(); + _temporaryDatabase = null; + } + /// /// Sets up the SQLiteCommand, setting up parameters when provided. /// Supports strings and floats from DMcode. @@ -39,33 +62,26 @@ public void SetupCommand(string command, ReadOnlySpan values) { for (var i = 0; i < values.Length; i++) { var arg = values[i]; - var type = arg.Type; - switch (type) { - case DreamValue.DreamValueType.String: - if (arg.TryGetValueAsString(out var stringValue)) { - _command.Parameters.AddWithValue($"@{i}", stringValue); - } - - break; - case DreamValue.DreamValueType.Float: - if (arg.TryGetValueAsFloat(out var floatValue)) { - _command.Parameters.AddWithValue($"@{i}", floatValue); - } - - break; - - case DreamValue.DreamValueType.DreamResource: - case DreamValue.DreamValueType.DreamObject: - case DreamValue.DreamValueType.DreamType: - case DreamValue.DreamValueType.DreamProc: - case DreamValue.DreamValueType.Appearance: - default: - // TODO: support saving BLOBS for icons, if we really want to - break; + if (arg.IsNull) { + _command.Parameters.AddWithValue($"@{i}", DBNull.Value); + } else if (arg.TryGetValueAsDreamResource(out var resource)) { + AddBlobParameter(i, resource.ResourceData); + } else if (arg.TryGetValueAsDreamObject(out var icon)) { + AddBlobParameter(i, icon.Icon.GenerateDMI().ResourceData); + } else if (arg.TryGetValueAsString(out var stringValue)) { + _command.Parameters.AddWithValue($"@{i}", stringValue); + } else if (arg.TryGetValueAsFloat(out var floatValue)) { + _command.Parameters.AddWithValue($"@{i}", floatValue); + } else { + throw new DMCrashRuntime("Invalid database query parameter"); } } } + private void AddBlobParameter(int index, byte[]? data) { + _command!.Parameters.Add($"@{index}", SqliteType.Blob).Value = data is null ? DBNull.Value : data; + } + /// /// Gets the names of all the columns in the current query /// @@ -84,26 +100,21 @@ public List GetAllColumns() { } /// - /// Gets the name of a single column in the current query + /// Gets the name of a selected column for /database/query.Columns(index). /// - /// The column ordinal value. - /// A of the name of the column. - public DreamValue GetColumn(int id) { - if (_reader is null) { - return DreamValue.Null; - } - - try { - var name = _reader.GetName(id); - return new DreamValue(name); - } catch (IndexOutOfRangeException exception) { - _errorCode = 1; - _errorMessage = exception.Message; + /// The one-based column index. + /// The column name, or an empty string for an invalid index. + public DreamValue GetColumnName(int column) { + if (_reader is null || column <= 0 || column > _reader.FieldCount) { + // BYOND returns an empty string for an invalid Columns() index. + return new DreamValue(string.Empty); } - return DreamValue.Null; + return new DreamValue(_reader.GetName(column - 1)); } + public bool HasCommand => _command != null; + public void ClearCommand() { _command?.Dispose(); _command = null; @@ -126,44 +137,49 @@ public void CloseReader() { /// Executes the currently held query against the SQLite database /// /// The that this query is being run against. - public void ExecuteCommand(DreamObjectDatabase database) { - if (!database.TryGetConnection(out var connection)) { - throw new DMCrashRuntime("Bad database"); + public bool ExecuteCommand(DreamObjectDatabase database) { + if (_command == null) { + return false; } - if (_command == null) { - return; + CloseReader(); + + if (!database.TryGetConnection(out var connection)) { + throw new DMCrashRuntime("Bad database"); } _command.Connection = connection; try { _reader = _command.ExecuteReader(); + return true; } catch (SqliteException exception) { _errorCode = exception.SqliteErrorCode; _errorMessage = exception.Message; database.SetError(exception.SqliteErrorCode, exception.Message); + return false; } } - public void NextRow() { - _reader?.Read(); + public bool NextRow() { + return _reader?.Read() ?? false; } /// - /// Attempts to fetch the value of a specific column. + /// Attempts to fetch the current row's value for /database/query.GetColumn(index). /// - /// The ordinal column number - /// The out variable to be populated with the of the result. - /// - public bool TryGetColumn(int column, out DreamValue value) { - if (_reader is null) { + /// The one-based column index. + /// The current row's column value. + /// True when the value was read; otherwise false. + public bool TryGetColumnValue(int column, out DreamValue value) { + if (_reader is null || column <= 0 || column > _reader.FieldCount) { + // BYOND returns null for an invalid GetColumn() index. value = DreamValue.Null; return false; } try { - value = GetDreamValueFromDbObject(_reader.GetValue(column)); + value = GetDreamValueFromDbObject(_reader.GetValue(column - 1)); return true; } catch (Exception exception) { _errorCode = 1; @@ -213,6 +229,9 @@ private static DreamValue GetDreamValueFromDbObject(object value) { long longValue => new DreamValue(longValue), int intValue => new DreamValue(intValue), string stringValue => new DreamValue(stringValue), + // BYOND's database API returns null, rather than exposing SQLite BLOB data. + byte[] => DreamValue.Null, + DBNull => DreamValue.Null, _ => throw new ArgumentOutOfRangeException(nameof(value)), }; } diff --git a/OpenDreamRuntime/Procs/Native/DreamProcNativeDatabaseQuery.cs b/OpenDreamRuntime/Procs/Native/DreamProcNativeDatabaseQuery.cs index 85dba06989..c925f886f5 100644 --- a/OpenDreamRuntime/Procs/Native/DreamProcNativeDatabaseQuery.cs +++ b/OpenDreamRuntime/Procs/Native/DreamProcNativeDatabaseQuery.cs @@ -13,10 +13,15 @@ public static DreamValue NativeProc_Add(NativeProc.Bundle bundle, DreamObject? s query.ClearCommand(); - if (!bundle.GetArgument(0, "text").TryGetValueAsString(out var command)) { + var commandArgument = bundle.GetArgument(0, "text"); + if (commandArgument.IsNull) { return DreamValue.Null; } + if (!commandArgument.TryGetValueAsString(out var command)) { + throw new DMCrashRuntime("Invalid database query text"); + } + query.SetupCommand(command, bundle.Arguments[1..]); return DreamValue.Null; @@ -46,7 +51,7 @@ public static DreamValue NativeProc_Columns(NativeProc.Bundle bundle, DreamObjec var query = (DreamObjectDatabaseQuery)src!; if (bundle.GetArgument(0, "column").TryGetValueAsInteger(out var column)) { - return query.GetColumn(column); + return query.GetColumnName(column); } var list = bundle.ObjectTree.CreateList(); @@ -76,16 +81,26 @@ public static DreamValue NativeProc_ErrorMsg(NativeProc.Bundle bundle, DreamObje } [DreamProc("Execute")] - [DreamProcParameter("database", Type = DreamValueTypeFlag.DreamObject)] + [DreamProcParameter("database", Type = DreamValueTypeFlag.String | DreamValueTypeFlag.DreamObject)] public static DreamValue NativeProc_Execute(NativeProc.Bundle bundle, DreamObject? src, DreamObject? usr) { var query = (DreamObjectDatabaseQuery)src!; - if (!bundle.GetArgument(0, "database").TryGetValueAsDreamObject(out DreamObjectDatabase? database)) - return DreamValue.Null; + var databaseArg = bundle.GetArgument(0, "database"); + if (!databaseArg.TryGetValueAsDreamObject(out DreamObjectDatabase? database)) { + // Execute(filename) executes against a database opened from the given file path. + if (!databaseArg.TryGetValueAsString(out var filename)) + throw new DMCrashRuntime("Bad database"); + + database = (DreamObjectDatabase)bundle.ObjectTree.CreateObject(bundle.ObjectTree.Database); + database.InitSpawn(new DreamProcArguments(new DreamValue(filename))); + query.SetTemporaryDatabase(database); + } - query.ExecuteCommand(database); + // BYOND treats Execute() on a query without a command as a no-op failure. + if (!query.HasCommand) + return new DreamValue(0); - return DreamValue.Null; + return new DreamValue(query.ExecuteCommand(database) ? 1 : 0); } [DreamProc("RowsAffected")] @@ -99,9 +114,8 @@ public static DreamValue NativeProc_RowsAffected(NativeProc.Bundle bundle, Dream public static DreamValue NativeProc_NextRow(NativeProc.Bundle bundle, DreamObject? src, DreamObject? usr) { var query = (DreamObjectDatabaseQuery)src!; - query.NextRow(); - - return DreamValue.Null; + // DM semantics: returns true while a row was read, false when exhausted. + return new DreamValue(query.NextRow() ? 1 : 0); } [DreamProc("GetColumn")] @@ -113,7 +127,7 @@ public static DreamValue NativeProc_GetColumn(NativeProc.Bundle bundle, DreamObj return DreamValue.Null; } - if (!query.TryGetColumn(column, out var value)) { + if (!query.TryGetColumnValue(column, out var value)) { return DreamValue.Null; }