mirror of
https://github.com/Mudlet/Mudlet
synced 2026-08-13 18:26:27 -04:00
fix: seven Lua library and mapper bugs found while speccing them (#9815)
Stacked on #9799, so the base is `fix-db-index-string` and this retargets to development once that merges. - `table.contains()` keeps a set of the tables it has walked, so a self-referential one (every Geyser object holds its container, which holds it back) answers instead of overflowing the stack, and `Geyser.Label:setDoubleClickCallback()` stores the `doubleClickCallback` key the label's own re-registration reads rather than one nothing reads. - db: a `UNIQUE` with no `ON CONFLICT` clause is now seen, so a change in uniqueness rebuilds the sheet; a sheet given as a list of column names takes the sheet options instead of swallowing `_index` as a phantom column; and an `_index` naming a column the sheet does not have is refused rather than quietly dropping the indexes the sheet already had. - `saveMap()` resolves a relative location against the profile directory the way `importMap()` does instead of against the directory Mudlet was started in, `loadMap()` looks in the same place, and a format version below the oldest one Mudlet can write is refused the way one that is too new already was. Test case: `lua local t = {} t.self = t display(table.contains(t, "x"))` answers `false` instead of raising, and `lua saveMap(42)` writes into `getMudletHomeDir()` rather than the directory Mudlet was started in. Worth knowing: `db:create` now hard-errors on an `_index` naming a column the sheet does not declare, where it used to load and silently lose the sheet's indexes. Closes #9777, Closes #9779, Closes #9780, Closes #9781, Closes #9782, Closes #9800, Closes #9801 Assisted-by: Claude:claude-opus-5
This commit is contained in:
parent
32834a5d57
commit
c2ec396fa2
9 changed files with 447 additions and 43 deletions
|
|
@ -1593,16 +1593,27 @@ void TMainConsole::finalize()
|
|||
// to the TMap class...?
|
||||
bool TMainConsole::saveMap(const QString& location, int saveVersion)
|
||||
{
|
||||
const QString filename_map =
|
||||
location.isEmpty() ? mudlet::getMudletPath(enums::profileDateTimeStampedMapPathFileName, mProfileName, QDateTime::currentDateTime().toString(qsl("yyyy-MM-dd#HH-mm-ss"))) : location;
|
||||
QString filename_map = location;
|
||||
if (filename_map.isEmpty()) {
|
||||
filename_map = mudlet::getMudletPath(enums::profileDateTimeStampedMapPathFileName, mProfileName, QDateTime::currentDateTime().toString(qsl("yyyy-MM-dd#HH-mm-ss")));
|
||||
} else if (const QFileInfo fileInfo(location); fileInfo.isRelative()) {
|
||||
// Resolve the name relative to the profile home directory the way
|
||||
// TMainConsole::importMap does, rather than against whatever directory
|
||||
// Mudlet happens to have been started in:
|
||||
filename_map = QDir::cleanPath(mudlet::getMudletPath(enums::profileDataItemPath, mProfileName, fileInfo.filePath()));
|
||||
}
|
||||
|
||||
const QDir dir_map(mudlet::getMudletPath(enums::profileMapsPath, mProfileName));
|
||||
if (!dir_map.exists() && !dir_map.mkpath(dir_map.path())) {
|
||||
qDebug().noquote() << "Error saving map: could not make the profile's map directory" << dir_map.path();
|
||||
return false;
|
||||
}
|
||||
|
||||
QSaveFile file_map(filename_map);
|
||||
if (!file_map.open(QIODevice::WriteOnly)) {
|
||||
// Naming the file matters more than usual: a relative location is not
|
||||
// the path the caller typed
|
||||
qDebug().noquote() << "Error saving map to" << filename_map << ":" << file_map.errorString();
|
||||
return false;
|
||||
}
|
||||
|
||||
|
|
@ -1651,12 +1662,19 @@ bool TMainConsole::loadMap(const QString& location)
|
|||
|
||||
pHost->mpMap->mapClear();
|
||||
|
||||
// The same resolution saveMap and importMap use, so that a map written
|
||||
// under a bare name is looked for where it was written:
|
||||
QString filePathName = location;
|
||||
if (const QFileInfo fileInfo(location); !location.isEmpty() && fileInfo.isRelative()) {
|
||||
filePathName = QDir::cleanPath(mudlet::getMudletPath(enums::profileDataItemPath, mProfileName, fileInfo.filePath()));
|
||||
}
|
||||
|
||||
qDebug() << "TMainConsole::loadMap() - restore map case 1.";
|
||||
pHost->mpMap->pushErrorMessagesToFile(tr("Pre-Map loading(1) report"), true);
|
||||
const QDateTime now(QDateTime::currentDateTime());
|
||||
|
||||
bool result = false;
|
||||
if (pHost->mpMap->restore(location)) {
|
||||
if (pHost->mpMap->restore(filePathName)) {
|
||||
pHost->mpMap->audit();
|
||||
pHost->mpMap->mpMapper->mp2dMap->init();
|
||||
pHost->mpMap->mpMapper->updateAreaComboBox();
|
||||
|
|
@ -1669,10 +1687,10 @@ bool TMainConsole::loadMap(const QString& location)
|
|||
pHost->mpMap->mpMapper->show();
|
||||
}
|
||||
|
||||
if (location.isEmpty()) {
|
||||
if (filePathName.isEmpty()) {
|
||||
pHost->mpMap->pushErrorMessagesToFile(tr("Loading map(1) at %1 report").arg(now.toString(Qt::ISODate)), true);
|
||||
} else {
|
||||
pHost->mpMap->pushErrorMessagesToFile(tr(R"(Loading map(1) "%1" at %2 report)").arg(location, now.toString(Qt::ISODate)), true);
|
||||
pHost->mpMap->pushErrorMessagesToFile(tr(R"(Loading map(1) "%1" at %2 report)").arg(filePathName, now.toString(Qt::ISODate)), true);
|
||||
}
|
||||
|
||||
pHost->mpMap->updateArea(-1);
|
||||
|
|
|
|||
15
src/TMap.cpp
15
src/TMap.cpp
|
|
@ -1112,11 +1112,7 @@ bool TMap::findPath(int from, int to)
|
|||
|
||||
bool TMap::serialize(QDataStream& ofs, int saveVersion)
|
||||
{
|
||||
// clamp version values
|
||||
if (saveVersion < 0) {
|
||||
saveVersion = 0;
|
||||
} else if (saveVersion > mMaxVersion) {
|
||||
saveVersion = mMaxVersion;
|
||||
if (saveVersion > mMaxVersion) {
|
||||
const QString errMsg = tr("[ ERROR ] - The format version \"%1\" you are trying to save the map with is too new\n"
|
||||
"for this version of Mudlet. Supported are only formats up to version %2.")
|
||||
.arg(QString::number(saveVersion), QString::number(mMaxVersion));
|
||||
|
|
@ -1124,6 +1120,15 @@ bool TMap::serialize(QDataStream& ofs, int saveVersion)
|
|||
postMessage(errMsg);
|
||||
return false;
|
||||
}
|
||||
if (saveVersion != 0 && saveVersion < mMinVersion) {
|
||||
//: Shown when a map save asks for a format version older than this Mudlet can write. %1 is the version asked for, %2 the oldest one supported.
|
||||
const QString errMsg = tr("[ ERROR ] - The format version \"%1\" you are trying to save the map with is too old\n"
|
||||
"for this version of Mudlet. Supported are only formats from version %2.")
|
||||
.arg(QString::number(saveVersion), QString::number(mMinVersion));
|
||||
appendErrorMsgWithNoLf(errMsg, false);
|
||||
postMessage(errMsg);
|
||||
return false;
|
||||
}
|
||||
|
||||
auto oldSaveVersion = mSaveVersion;
|
||||
|
||||
|
|
|
|||
|
|
@ -117,8 +117,11 @@ function db:_sql_columns(value)
|
|||
|
||||
if t == "table" then
|
||||
for _, v in ipairs(value) do
|
||||
-- see https://www.sqlite.org/syntaxdiagrams.html#ordering-term
|
||||
if v:lower() == "desc" or v:lower() == "asc" then
|
||||
assert(type(v) == "string", "Column names must be strings, not " .. type(v) .. ".")
|
||||
-- see https://www.sqlite.org/syntaxdiagrams.html#ordering-term: a sort
|
||||
-- direction belongs to the column in front of it, so one that leads the
|
||||
-- list can only be a column of that name
|
||||
if col_chunks[1] and (v:lower() == "desc" or v:lower() == "asc") then
|
||||
col_chunks[#col_chunks] = col_chunks[#col_chunks] .. " " .. v
|
||||
else
|
||||
col_chunks[#col_chunks + 1] = '"' .. v:lower() .. '"'
|
||||
|
|
@ -359,6 +362,9 @@ end
|
|||
--- Note that you have to use double {{ }} if you have composite index/unique constrain.
|
||||
--- A single column may be given on its own instead of in a list, so _index = "city"
|
||||
--- and _unique = "name" mean the same as the two lines above.
|
||||
--- A sheet may also be given as a plain list of its column names, which then all
|
||||
--- hold text and default to "". The sheet options are keys rather than list members,
|
||||
--- so they work there too: enemies = {"name", "city", _index = "city"}
|
||||
function db:create(db_name, sheets, force)
|
||||
if not db.__env or db.__env == 'SQLite3 environment (closed)' then
|
||||
db.__env = luasql.sqlite3()
|
||||
|
|
@ -380,9 +386,28 @@ function db:create(db_name, sheets, force)
|
|||
|
||||
-- the sheet was provided in {"column1", "column2"} format
|
||||
if sheet[1] ~= nil then
|
||||
-- assume field types are text, and should default to ""
|
||||
for _, col_name in pairs(sheet) do
|
||||
columns[col_name] = ""
|
||||
-- The list holds the column names, which are text defaulting to "". A key
|
||||
-- is a sheet option when it starts with an underscore and an error
|
||||
-- otherwise: sweeping keys in with the column names would make a column
|
||||
-- out of an index definition. Numeric keys are checked against #sheet so
|
||||
-- that a stray [7] in a two-item list is not taken for a column name.
|
||||
local column_count = #sheet
|
||||
for key, value in pairs(sheet) do
|
||||
if type(key) == "number" and key % 1 == 0 and key >= 1 and key <= column_count then
|
||||
if type(value) == "string" then
|
||||
columns[value] = ""
|
||||
else
|
||||
is_valid = false
|
||||
table.insert(msgs, "db:create - "..sheet_name.." - column name #"..key..
|
||||
" is a "..type(value)..", but a sheet's column names have to be strings.")
|
||||
end
|
||||
elseif type(key) == "string" and string.starts(key, "_") then
|
||||
options[key] = value
|
||||
else
|
||||
is_valid = false
|
||||
table.insert(msgs, "db:create - "..sheet_name.." - "..tostring(key)..
|
||||
" is neither one of the sheet's column names nor a sheet option: a sheet is either a list of column names or a table of column names and their default values.")
|
||||
end
|
||||
end
|
||||
|
||||
-- sheet provided in {"column1" = default} format
|
||||
|
|
@ -432,6 +457,35 @@ function db:create(db_name, sheets, force)
|
|||
if type(options._index) == "string" then
|
||||
options._index = { options._index }
|
||||
end
|
||||
|
||||
-- An index on a column this sheet does not declare is refused rather than
|
||||
-- carried: db:_migrate_indexes cannot make the index a typo asks for, and
|
||||
-- a typo that replaced the only entry leaves db:_drop_orphaned_indexes
|
||||
-- treating it as the whole wanted set, dropping the index the sheet did
|
||||
-- have. The shapes _validate_index refused above are left to it to report.
|
||||
if type(options._index) == "table" then
|
||||
for _, index_entry in ipairs(options._index) do
|
||||
local index_columns = type(index_entry) == "table" and index_entry or {index_entry}
|
||||
for _, column_name in ipairs(index_columns) do
|
||||
if type(column_name) == "string" and columns[column_name] == nil then
|
||||
local lowered = column_name:lower()
|
||||
is_valid = false
|
||||
if column_name == "_row_id" then
|
||||
table.insert(msgs, "db:create - "..sheet_name.." - _index names \"_row_id\", which is the "..
|
||||
"key every sheet is given rather than one of its own columns.")
|
||||
elseif lowered == "asc" or lowered == "desc" then
|
||||
-- db:_sql_columns would build the ordering term, but
|
||||
-- db:_index_valid refuses it, so the index was never made
|
||||
table.insert(msgs, "db:create - "..sheet_name.." - _index names \""..column_name..
|
||||
"\", and an index takes column names only, not a sort direction.")
|
||||
else
|
||||
table.insert(msgs, "db:create - "..sheet_name.." - _index names \""..column_name..
|
||||
"\", which is not one of the sheet's columns.")
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
schema[sheet_name] = { columns = columns, options = options }
|
||||
|
|
@ -466,9 +520,10 @@ end
|
|||
|
||||
|
||||
-- NOT LUADOC
|
||||
-- Extracts UNIQUE constraints with ON CONFLICT clauses from a CREATE TABLE statement.
|
||||
-- Extracts UNIQUE constraints from a CREATE TABLE statement.
|
||||
-- This includes both column-level constraints (e.g., "col1" TEXT UNIQUE ON CONFLICT REPLACE)
|
||||
-- and table-level constraints (e.g., UNIQUE("col1", "col2") ON CONFLICT FAIL).
|
||||
-- and table-level constraints (e.g., UNIQUE("col1", "col2") ON CONFLICT FAIL), each of
|
||||
-- which may come without its ON CONFLICT clause (e.g., "col1" TEXT UNIQUE).
|
||||
-- This allows us to detect when constraint definitions have changed without being affected by
|
||||
-- column additions/removals.
|
||||
function db:_extract_table_constraints(sql)
|
||||
|
|
@ -487,17 +542,48 @@ function db:_extract_table_constraints(sql)
|
|||
|
||||
local constraints = {}
|
||||
|
||||
-- Find table-level UNIQUE constraints
|
||||
-- They look like: UNIQUE("col1") ON CONFLICT REPLACE or UNIQUE("col1", "col2") ON CONFLICT FAIL
|
||||
for constraint in content:gmatch('unique%s*%([^)]+%)%s+on%s+conflict%s+%w+') do
|
||||
table.insert(constraints, constraint)
|
||||
-- A column name and a default value are both quoted, and either can hold the
|
||||
-- word, so the search runs over a copy with the quoted parts blanked out.
|
||||
-- Same-length blanks keep every offset lined up with the content itself.
|
||||
local function blank(quoted)
|
||||
return (" "):rep(#quoted)
|
||||
end
|
||||
local searchable = content:gsub('"[^"]*"', blank)
|
||||
searchable = searchable:gsub("'[^']*'", blank)
|
||||
|
||||
-- Find column-level UNIQUE constraints
|
||||
-- They look like: "col1" TEXT NULL DEFAULT "" UNIQUE ON CONFLICT REPLACE
|
||||
-- We need to extract just the "UNIQUE ON CONFLICT X" part for comparison
|
||||
for constraint in content:gmatch('unique%s+on%s+conflict%s+%w+') do
|
||||
table.insert(constraints, constraint)
|
||||
-- Each UNIQUE is picked up with the column list it may carry, then with the
|
||||
-- ON CONFLICT clause it may carry. Both parts are optional: SQLite defaults
|
||||
-- the conflict resolution to ABORT, so a sheet whose table was not written by
|
||||
-- this module can hold a bare UNIQUE, and a bare one has to be seen or a
|
||||
-- change in uniqueness compares equal to no uniqueness at all.
|
||||
local position = 1
|
||||
while true do
|
||||
local start, stop = searchable:find("unique", position, true)
|
||||
if not start then
|
||||
break
|
||||
end
|
||||
position = stop + 1
|
||||
|
||||
-- and a column called unique_id is not one either
|
||||
local before = start > 1 and searchable:sub(start - 1, start - 1) or " "
|
||||
local after = searchable:sub(stop + 1, stop + 1)
|
||||
if not before:match("[%w_]") and not after:match("[%w_]") then
|
||||
local constraint = "unique"
|
||||
|
||||
local columns_start, columns_stop = content:find("^%s*%([^)]+%)", position)
|
||||
if columns_start then
|
||||
constraint = constraint .. content:sub(columns_start, columns_stop)
|
||||
position = columns_stop + 1
|
||||
end
|
||||
|
||||
local conflict_start, conflict_stop = content:find("^%s+on%s+conflict%s+%w+", position)
|
||||
if conflict_start then
|
||||
constraint = constraint .. content:sub(conflict_start, conflict_stop)
|
||||
position = conflict_stop + 1
|
||||
end
|
||||
|
||||
table.insert(constraints, constraint)
|
||||
end
|
||||
end
|
||||
|
||||
-- Sort for consistent comparison
|
||||
|
|
|
|||
|
|
@ -190,11 +190,15 @@ end
|
|||
|
||||
|
||||
|
||||
--- Determines if a table contains a value as a key or as a value (recursive).
|
||||
function table._contains(t, value)
|
||||
if type(t) ~= "table" then
|
||||
return nil, "first parameter passed isn't a table"
|
||||
-- Tables that reach themselves are ordinary here: every Geyser object holds its
|
||||
-- container, which holds it back again, so the descent has to remember where it
|
||||
-- has been or it never ends. The set is kept out of table._contains' own
|
||||
-- signature, which ignores anything past the value to look for.
|
||||
local function containsValue(t, value, seen)
|
||||
if seen[t] then
|
||||
return false
|
||||
end
|
||||
seen[t] = true
|
||||
|
||||
for k, v in pairs(t) do
|
||||
if v == value then
|
||||
|
|
@ -202,7 +206,7 @@ function table._contains(t, value)
|
|||
elseif k == value then
|
||||
return true
|
||||
elseif type(v) == "table" then
|
||||
if table.contains(v, value) then
|
||||
if containsValue(v, value, seen) then
|
||||
return true
|
||||
end
|
||||
end
|
||||
|
|
@ -210,6 +214,15 @@ function table._contains(t, value)
|
|||
return false
|
||||
end
|
||||
|
||||
--- Determines if a table contains a value as a key or as a value (recursive).
|
||||
function table._contains(t, value)
|
||||
if type(t) ~= "table" then
|
||||
return nil, "first parameter passed isn't a table"
|
||||
end
|
||||
|
||||
return containsValue(t, value, {})
|
||||
end
|
||||
|
||||
function table.contains(tbl, ...)
|
||||
for _,item in ipairs({...}) do
|
||||
if table._contains(tbl, item) then return true end
|
||||
|
|
|
|||
|
|
@ -514,8 +514,8 @@ end
|
|||
-- @param ... Parameters to pass to the function. Must be strings or numbers.
|
||||
function Geyser.Label:setDoubleClickCallback (func, ...)
|
||||
setLabelDoubleClickCallback(self.name, func, ...)
|
||||
self.doubleclickCallback = func
|
||||
self.doubleclickArgs = { ... }
|
||||
self.doubleClickCallback = func
|
||||
self.doubleClickArgs = { ... }
|
||||
end
|
||||
|
||||
--- Sets a callback to be used when a mouse click is released over this label. When this
|
||||
|
|
|
|||
|
|
@ -2338,6 +2338,19 @@ describe("Tests db's internal SQL helpers", function()
|
|||
assert.is_false(ok)
|
||||
assert.is_truthy(string.find(err, "Must specify either a table array or string for index, not number", 1, true))
|
||||
end)
|
||||
|
||||
it("refuses a list member that is not a string", function()
|
||||
local ok, err = pcall(function() return db:_sql_columns({42}) end)
|
||||
assert.is_false(ok)
|
||||
assert.is_truthy(string.find(err, "Column names must be strings, not number", 1, true))
|
||||
end)
|
||||
|
||||
it("quotes a leading sort direction as the column name it has to be", function()
|
||||
-- there is no column in front of it to attach it to, and a sheet is
|
||||
-- allowed a column called desc
|
||||
assert.are.equal('"desc"', db:_sql_columns({"desc"}))
|
||||
assert.are.equal('"asc","name"', db:_sql_columns({"asc", "name"}))
|
||||
end)
|
||||
end)
|
||||
|
||||
describe("Tests db:_sql_fields", function()
|
||||
|
|
@ -2581,6 +2594,34 @@ describe("Tests db's internal SQL helpers", function()
|
|||
local after = 'CREATE TABLE people ("name" TEXT UNIQUE ON CONFLICT FAIL, "city" TEXT NULL DEFAULT "")'
|
||||
assert.are.equal(db:_extract_table_constraints(before), db:_extract_table_constraints(after))
|
||||
end)
|
||||
|
||||
it("sees a UNIQUE that carries no ON CONFLICT clause", function()
|
||||
-- sqlite defaults the conflict resolution to ABORT, so a table this
|
||||
-- module did not write can hold one of these; missing it makes a sheet
|
||||
-- with a unique constraint compare equal to one without
|
||||
assert.are.equal("unique",
|
||||
db:_extract_table_constraints('CREATE TABLE people ("name" TEXT NULL DEFAULT "" UNIQUE, "city" TEXT NULL)'))
|
||||
assert.are.equal('unique("name", "city")',
|
||||
db:_extract_table_constraints('CREATE TABLE people ("name" TEXT NULL, "city" TEXT NULL, UNIQUE("name", "city"))'))
|
||||
end)
|
||||
|
||||
it("tells a bare UNIQUE apart from one with a conflict clause", function()
|
||||
local bare = 'CREATE TABLE people ("name" TEXT UNIQUE)'
|
||||
local resolved = 'CREATE TABLE people ("name" TEXT UNIQUE ON CONFLICT FAIL)'
|
||||
assert.are_not.equal(db:_extract_table_constraints(bare), db:_extract_table_constraints(resolved))
|
||||
assert.are_not.equal(db:_extract_table_constraints(bare), db:_extract_table_constraints('CREATE TABLE people ("name" TEXT)'))
|
||||
end)
|
||||
|
||||
it("does not mistake a column named after the keyword for a constraint", function()
|
||||
assert.are.equal("", db:_extract_table_constraints('CREATE TABLE people ("unique_id" TEXT NULL DEFAULT "")'))
|
||||
assert.are.equal("", db:_extract_table_constraints('CREATE TABLE people ("uniqueness" TEXT NULL DEFAULT "")'))
|
||||
assert.are.equal("", db:_extract_table_constraints('CREATE TABLE people ("unique" TEXT NULL DEFAULT "")'))
|
||||
assert.are.equal("", db:_extract_table_constraints('CREATE TABLE people ("kind" TEXT NULL DEFAULT "unique")'))
|
||||
assert.are.equal("", db:_extract_table_constraints('CREATE TABLE people ("kind" TEXT NULL DEFAULT "a unique sword")'))
|
||||
-- and a column that is both named after the keyword and carries one
|
||||
assert.are.equal("unique on conflict fail",
|
||||
db:_extract_table_constraints('CREATE TABLE people ("unique" TEXT NULL DEFAULT "" UNIQUE ON CONFLICT FAIL)'))
|
||||
end)
|
||||
end)
|
||||
|
||||
describe("Tests db:_build_create_table_sql", function()
|
||||
|
|
@ -2879,6 +2920,34 @@ describe("Tests db's internals against a real database", function()
|
|||
assert.is_nil(rows[1].city)
|
||||
end)
|
||||
|
||||
it("rebuilds a sheet whose UNIQUE carries no conflict clause", function()
|
||||
-- sqlite defaults the conflict resolution to ABORT, so a sheet that this
|
||||
-- module did not write can hold a bare UNIQUE. Dropping _unique from the
|
||||
-- schema then has to rebuild the table, which it only does if the bare
|
||||
-- constraint is seen in the first place
|
||||
local schema = db.__schema[dbName].people
|
||||
local conn = db.__conn[dbName]
|
||||
|
||||
schema.options._unique = {"name"}
|
||||
local legacy = db:_build_create_table_sql(schema, "people"):gsub(" ON CONFLICT %u+", "")
|
||||
assert.is_truthy(string.find(legacy, '"name" TEXT NULL DEFAULT "" UNIQUE', 1, true))
|
||||
conn:execute("DROP TABLE people")
|
||||
conn:execute(legacy)
|
||||
conn:commit()
|
||||
|
||||
assert.is_true(db:add(mydb.people, {name = "Bob", city = "Ankh-Morpork"}))
|
||||
|
||||
schema.options._unique = nil
|
||||
db:_migrate(dbName, "people")
|
||||
|
||||
-- the uniqueness the schema no longer asks for is gone, and the row that
|
||||
-- was there came through the rebuild
|
||||
assert.is_true(db:add(mydb.people, {name = "Bob", city = "Lancre"}))
|
||||
local rows = db:fetch(db:get_database(dbName).people)
|
||||
assert.are.equal(2, #rows)
|
||||
assert.are.equal("Bob", rows[1].name)
|
||||
end)
|
||||
|
||||
it("creates the indexes the schema asks for", function()
|
||||
local conn = db.__conn[dbName]
|
||||
conn:execute("DROP INDEX IF EXISTS " .. db:_index_name("people", "city"))
|
||||
|
|
@ -3107,15 +3176,140 @@ describe("Tests db:create with a single column name as _index", function()
|
|||
assert.are.equal("Bob", rows[1].name)
|
||||
end)
|
||||
|
||||
it("makes no index at all for a column the sheet does not have", function()
|
||||
-- db:_index_valid refuses the column quietly rather than raising, so a
|
||||
-- typo in the string costs the index and the ones that were there before
|
||||
it("refuses a column the sheet does not have, keeping the indexes it had", function()
|
||||
-- an index on a column that is not there can never be created, and taking
|
||||
-- the typo for the wanted set would drop the indexes the sheet did have
|
||||
db:create(dbName, {people = {name = "", city = "", _index = "city"}})
|
||||
assert.are.equal(1, #indexNames("people"))
|
||||
db:close(dbName)
|
||||
|
||||
db:create(dbName, {people = {name = "", city = "", _index = "citty"}})
|
||||
assert.are.same({}, indexNames("people"))
|
||||
local ok, err = pcall(function()
|
||||
db:create(dbName, {people = {name = "", city = "", _index = "citty"}})
|
||||
end)
|
||||
assert.is_false(ok)
|
||||
assert.is_truthy(string.find(err, '_index names "citty", which is not one of the sheet\'s columns', 1, true))
|
||||
assert.are.same({db:_index_name("people", "city")}, indexNames("people"))
|
||||
end)
|
||||
|
||||
it("refuses a typo in a list or a compound index too", function()
|
||||
local ok, err = pcall(function()
|
||||
db:create(dbName, {people = {name = "", city = "", _index = {"city", "citty"}}})
|
||||
end)
|
||||
assert.is_false(ok)
|
||||
assert.is_truthy(string.find(err, '_index names "citty"', 1, true))
|
||||
|
||||
ok, err = pcall(function()
|
||||
db:create(dbName, {people = {name = "", city = "", _index = {{"city", "citty"}}}})
|
||||
end)
|
||||
assert.is_false(ok)
|
||||
assert.is_truthy(string.find(err, '_index names "citty"', 1, true))
|
||||
end)
|
||||
|
||||
it("refuses a sort direction where a column name belongs", function()
|
||||
-- db:_sql_columns would render "name" desc, but db:_index_valid refuses the
|
||||
-- entry, so an index with a sort direction has never been created: saying so
|
||||
-- beats leaving the sheet with no index and no complaint
|
||||
local ok, err = pcall(function()
|
||||
db:create(dbName, {people = {name = "", city = "", _index = {{"name", "desc"}}}})
|
||||
end)
|
||||
assert.is_false(ok)
|
||||
assert.is_truthy(string.find(err, "an index takes column names only, not a sort direction", 1, true))
|
||||
|
||||
-- a sheet that really has a column of that name is not refused; what
|
||||
-- db:_sql_columns then makes of it is that function's business
|
||||
assert.is_table(db:create(dbName, {people = {name = "", desc = "", _index = {{"name", "desc"}}}}))
|
||||
end)
|
||||
|
||||
it("refuses the _row_id no sheet definition names either", function()
|
||||
-- the sheet is given one, but no definition declares it: it is not there to
|
||||
-- index on the db:create that makes the sheet, and db:_drop_orphaned_indexes
|
||||
-- cannot match the leading underscore, so an index on it was dropped and
|
||||
-- made again on every db:create after that
|
||||
local ok, err = pcall(function()
|
||||
db:create(dbName, {people = {name = "", city = "", _index = "_row_id"}})
|
||||
end)
|
||||
assert.is_false(ok)
|
||||
assert.is_truthy(string.find(err, '_index names "_row_id"', 1, true))
|
||||
end)
|
||||
end)
|
||||
|
||||
-- A sheet may be given as a list of its column names instead of a table of
|
||||
-- names and defaults. The two forms take the same sheet options, which are keys
|
||||
-- rather than list members in both.
|
||||
describe("Tests db:create with a sheet given as a list of column names", function()
|
||||
local dbName = "indexarrayformtestingonly"
|
||||
local dbFile = getMudletHomeDir() .. "/Database_" .. dbName .. ".db"
|
||||
|
||||
local function indexNames(sheetName)
|
||||
local conn = db.__conn[dbName]
|
||||
local cursor = conn:execute(
|
||||
"SELECT name FROM sqlite_master WHERE type = 'index' AND tbl_name = '" .. sheetName .. "' AND sql IS NOT NULL"
|
||||
)
|
||||
local names = {}
|
||||
local row = cursor:fetch({}, "a")
|
||||
while row do
|
||||
names[#names + 1] = row.name
|
||||
row = cursor:fetch({}, "a")
|
||||
end
|
||||
cursor:close()
|
||||
table.sort(names)
|
||||
return names
|
||||
end
|
||||
|
||||
after_each(function()
|
||||
if not pcall(function() db:close(dbName) end) then
|
||||
db.__conn[dbName] = nil
|
||||
end
|
||||
os.remove(dbFile)
|
||||
end)
|
||||
|
||||
it("takes the listed names as the columns, with no options among them", function()
|
||||
local mydb = db:create(dbName, {people = {"name", "city", _index = "city"}})
|
||||
assert.are.same({city = "", name = ""}, db.__schema[dbName].people.columns)
|
||||
assert.is_true(db:add(mydb.people, {name = "Bob", city = "Ankh-Morpork"}))
|
||||
assert.are.equal("Bob", db:fetch(mydb.people)[1].name)
|
||||
end)
|
||||
|
||||
it("creates the index the list form asked for", function()
|
||||
db:create(dbName, {people = {"name", "city", _index = "city"}})
|
||||
assert.are.same({db:_index_name("people", "city")}, indexNames("people"))
|
||||
end)
|
||||
|
||||
it("takes a list of index columns rather than raising", function()
|
||||
local ok, err = pcall(function()
|
||||
db:create(dbName, {people = {"name", "city", _index = {"city"}}})
|
||||
end)
|
||||
assert.is_true(ok, tostring(err))
|
||||
assert.are.same({db:_index_name("people", "city")}, indexNames("people"))
|
||||
end)
|
||||
|
||||
it("takes _unique and _violations from the list form as well", function()
|
||||
local mydb = db:create(dbName, {people = {"name", "city", _unique = "name", _violations = "IGNORE"}})
|
||||
assert.are.equal("IGNORE", db.__schema[dbName].people.options._violations)
|
||||
-- and no column named after an option's value
|
||||
assert.are.same({city = "", name = ""}, db.__schema[dbName].people.columns)
|
||||
assert.is_true(db:add(mydb.people, {name = "Bob", city = "Ankh-Morpork"}))
|
||||
-- IGNORE rather than the default FAIL, so the second one is dropped quietly
|
||||
assert.is_true(db:add(mydb.people, {name = "Bob", city = "Lancre"}))
|
||||
assert.are.equal(1, #db:fetch(mydb.people))
|
||||
end)
|
||||
|
||||
it("refuses a key that is neither a column name nor a sheet option", function()
|
||||
-- neither form on its own: "city" is keyed, and it is not a sheet option
|
||||
local ok, err = pcall(function()
|
||||
db:create(dbName, {people = {"name", city = ""}})
|
||||
end)
|
||||
assert.is_false(ok)
|
||||
assert.is_truthy(string.find(err, "city is neither one of the sheet's column names nor a sheet option", 1, true))
|
||||
end)
|
||||
|
||||
it("refuses a listed column name that is not a string", function()
|
||||
-- it would otherwise reach the CREATE TABLE build, which raises from inside
|
||||
-- string.format naming neither the sheet nor the column
|
||||
local ok, err = pcall(function()
|
||||
db:create(dbName, {people = {"name", true}})
|
||||
end)
|
||||
assert.is_false(ok)
|
||||
assert.is_truthy(string.find(err, "column name #2 is a boolean", 1, true))
|
||||
end)
|
||||
end)
|
||||
|
||||
|
|
|
|||
|
|
@ -762,22 +762,21 @@ describe("Tests Geyser.Label movies, callbacks and nesting", function()
|
|||
assert.are.same({}, label.clickArgs)
|
||||
end)
|
||||
|
||||
-- setDoubleClickCallback is missing from the readback below on purpose: it
|
||||
-- stores self.doubleclickCallback/doubleclickArgs while the constructor and
|
||||
-- every other setter use the doubleClickCallback/doubleClickArgs spelling,
|
||||
-- so there is nothing here worth freezing until that is settled
|
||||
it("remembers what the other callbacks registered too", function()
|
||||
local handler = function() end
|
||||
label:setDoubleClickCallback(handler, "d")
|
||||
label:setReleaseCallback(handler, "r")
|
||||
label:setMoveCallback(handler, "m")
|
||||
label:setWheelCallback(handler, "w")
|
||||
label:setOnEnter(handler, "e")
|
||||
label:setOnLeave(handler, "l")
|
||||
assert.are.same({"d"}, label.doubleClickArgs)
|
||||
assert.are.same({"r"}, label.releaseArgs)
|
||||
assert.are.same({"m"}, label.moveArgs)
|
||||
assert.are.same({"w"}, label.wheelArgs)
|
||||
assert.are.same({"e"}, label.onEnterArgs)
|
||||
assert.are.same({"l"}, label.onLeaveArgs)
|
||||
assert.are.equal(handler, label.doubleClickCallback)
|
||||
assert.are.equal(handler, label.releaseCallback)
|
||||
assert.are.equal(handler, label.moveCallback)
|
||||
assert.are.equal(handler, label.wheelCallback)
|
||||
|
|
@ -794,13 +793,28 @@ describe("Tests Geyser.Label movies, callbacks and nesting", function()
|
|||
local built = track(Geyser.Label:new({
|
||||
name = "glnConsCallback", x = 0, y = 0, width = 60, height = 40,
|
||||
clickCallback = "echo", clickArgs = {"hello"},
|
||||
doubleClickCallback = "echo", doubleClickArgs = "hello",
|
||||
onEnter = "echo", onEnterArgs = "hello",
|
||||
}, container))
|
||||
assert.are.equal("echo", built.clickCallback)
|
||||
assert.are.same({"hello"}, built.clickArgs)
|
||||
assert.are.equal("echo", built.doubleClickCallback)
|
||||
assert.are.same({"hello"}, built.doubleClickArgs)
|
||||
assert.are.equal("echo", built.onEnter)
|
||||
assert.are.same({"hello"}, built.onEnterArgs)
|
||||
end)
|
||||
|
||||
it("hands a double-click callback back to the constructor's spelling", function()
|
||||
-- the constructor and Geyser.Label:new's re-registration only look at
|
||||
-- doubleClickCallback, so a setter that stored any other key would leave
|
||||
-- a label that forgets its handler the moment it is rebuilt
|
||||
local handler = function() end
|
||||
label:setDoubleClickCallback(handler, "a", 2)
|
||||
assert.are.equal(handler, label.doubleClickCallback)
|
||||
assert.are.same({"a", 2}, label.doubleClickArgs)
|
||||
assert.is_nil(label.doubleclickCallback)
|
||||
assert.is_nil(label.doubleclickArgs)
|
||||
end)
|
||||
end)
|
||||
|
||||
describe("Geyser.Label:addChild", function()
|
||||
|
|
|
|||
|
|
@ -2065,6 +2065,50 @@ describe("Tests saveMap and loadMap", function()
|
|||
end)
|
||||
assert.is_false(saveMap(savePath, 9999))
|
||||
end)
|
||||
|
||||
it("refuses a format version older than this Mudlet can write", function()
|
||||
finally(function()
|
||||
saveMap(savePath)
|
||||
os.remove(savePath)
|
||||
end)
|
||||
-- 16 is one below the oldest format this Mudlet writes, and a refusal
|
||||
-- has to be as flat as the one for a version that is too new
|
||||
assert.is_false(saveMap(savePath, 16))
|
||||
assert.is_false(saveMap(savePath, -1))
|
||||
end)
|
||||
|
||||
it("resolves a relative location against the profile directory", function()
|
||||
-- and not against the directory Mudlet happens to have been started in,
|
||||
-- which for a spec run is the build or source tree
|
||||
local relative = "mapper_spec_relative.dat"
|
||||
local function clear()
|
||||
os.remove(getMudletHomeDir() .. "/" .. relative)
|
||||
os.remove(relative)
|
||||
end
|
||||
clear()
|
||||
finally(clear)
|
||||
|
||||
assert.is_true(saveMap(relative))
|
||||
assert.is_true(io.exists(getMudletHomeDir() .. "/" .. relative))
|
||||
assert.is_false(io.exists(relative))
|
||||
-- and loadMap has to look in the same place, or a map saved under a bare
|
||||
-- name cannot be loaded back under it
|
||||
assert.is_true(loadMap(relative))
|
||||
end)
|
||||
|
||||
it("resolves a number the same way, Lua having made a name out of it", function()
|
||||
local numbered = "42"
|
||||
local function clear()
|
||||
os.remove(getMudletHomeDir() .. "/" .. numbered)
|
||||
os.remove(numbered)
|
||||
end
|
||||
clear()
|
||||
finally(clear)
|
||||
|
||||
assert.is_true(saveMap(42))
|
||||
assert.is_true(io.exists(getMudletHomeDir() .. "/" .. numbered))
|
||||
assert.is_false(io.exists(numbered))
|
||||
end)
|
||||
end)
|
||||
|
||||
-- Careful with the order of anything added here: a load that fails still
|
||||
|
|
|
|||
|
|
@ -532,6 +532,36 @@ describe("Tests TableUtils.lua functions", function()
|
|||
assert.is_false(table.contains(tbl, "five"))
|
||||
end)
|
||||
|
||||
it("should cope with a table that holds itself", function()
|
||||
local tbl = {one = 1}
|
||||
tbl.self = tbl
|
||||
assert.is_true(table.contains(tbl, "one"))
|
||||
assert.is_false(table.contains(tbl, "five"))
|
||||
end)
|
||||
|
||||
it("should cope with a cycle between two tables", function()
|
||||
local first, second = {}, {}
|
||||
first.second = second
|
||||
second.first = first
|
||||
second.needle = "found me"
|
||||
assert.is_true(table.contains(first, "found me"))
|
||||
assert.is_false(table.contains(first, "not in here"))
|
||||
end)
|
||||
|
||||
it("should cope with a Geyser object, which always holds itself", function()
|
||||
-- a label knows its container and the container's windowList knows the
|
||||
-- label, so this is the cycle ordinary scripts hit
|
||||
local label = Geyser.Label:new({
|
||||
name = "tableUtilsSpecCycleLabel", x = 0, y = 0, width = 50, height = 20,
|
||||
})
|
||||
finally(function() label:delete() end)
|
||||
-- the search below is only worth anything while that is really a cycle
|
||||
assert.are.equal(label, label.container.windowList[label.name])
|
||||
|
||||
assert.is_true(table.contains(label, "tableUtilsSpecCycleLabel"))
|
||||
assert.is_false(table.contains(label, "no Geyser object holds this"))
|
||||
end)
|
||||
|
||||
end)
|
||||
|
||||
-- table.contains is a loop over table._contains, one pass per value it was
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue