fix(model): falsy values and soft-deleted rows leaking through the model layer

src/model.lua

@@ -89,7 +89,11 @@ function NormRecord:__init(model, row, persisted)
             local col = model.columns[i];
             local value = row[col.name];
             if (value ~= nil) then
-                self[col.name] = self.__persisted and model:parse(col, value) or value;
+                if (self.__persisted) then
+                    self[col.name] = model:parse(col, value);
+                else
+                    self[col.name] = value;
+                end
             end
         end
     end
@@ -1131,6 +1135,7 @@ function NormModel:_find_by_attrs(attributes, cb)
     for _, k in ipairs(utils.sorted_keys(attributes)) do
         state.wheres[#state.wheres + 1] = { column = k, op = "=", value = attributes[k] };
     end
+    utils.soft_scope(state, self); -- a trashed row must not satisfy a find_or_* lookup
     local statement, params = sqlmod.select(state, d);
     orm:_trace(statement, params);
     orm:_raw_query(statement, params, function(err, rows)
@@ -1247,6 +1252,15 @@ function NormModel:upsert(data, opts)
     utils.assert(type(conflict) == "table" and #conflict > 0,
         ("upsert on '%s' needs conflict columns (opts.conflict) or a primary key"):format(model.table));
 
+    -- The row is read back by the conflict columns, so a conflict column missing
+    -- from `data` would compile to `WHERE <col> IS NULL` and resolve nil even
+    -- though the write succeeded. That happens by default whenever the model has
+    -- an auto-increment primary key and the caller does not supply it.
+    for _, c in ipairs(conflict) do
+        utils.assert(data[c] ~= nil,
+            ("upsert on '%s': conflict column '%s' is missing from the data"):format(model.table, c));
+    end
+
     -- write payload (+ timestamps for the INSERT branch).
     local write = {};
     for k, v in pairs(data) do write[k] = v; end
@@ -1283,6 +1297,7 @@ function NormModel:upsert(data, opts)
             for _, c in ipairs(conflict) do
                 state.wheres[#state.wheres + 1] = { column = c, op = "=", value = write[c] };
             end
+            utils.soft_scope(state, model);
             local sel, sparams = sqlmod.select(state, d);
             orm:_trace(sel, sparams);
             orm:_raw_query(sel, sparams, function(serr, rows)

src/orm.lua

@@ -416,9 +416,13 @@ function NormOrm:_load_include_batch(model, mains, name, spec, cb)
         if (v ~= nil and not seen[v]) then seen[v] = true; keys[#keys + 1] = v; end
     end
 
-    local empty = (rel.kind == "has_many" or rel.kind == "belongs_to_many") and {} or nil;
+    local wants_list = (rel.kind == "has_many" or rel.kind == "belongs_to_many");
     if (#keys == 0) then
-        for i = 1, #mains do mains[i][name] = empty; end
+        -- A fresh table per parent: one shared table would make a mutation on any
+        -- parent's collection visible on all the others.
+        for i = 1, #mains do
+            if (wants_list) then mains[i][name] = {}; else mains[i][name] = nil; end
+        end
         return cb();
     end
 
@@ -469,7 +473,11 @@ function NormOrm:_load_include_batch(model, mains, name, spec, cb)
         for i = 1, #mains do
             local g = groups[mains[i][source_key]] or {};
             if (spec and spec.limit and rel.kind == "has_many") then g = slice(g, spec.offset, spec.limit); end
-            mains[i][name] = (rel.kind == "has_one") and (g[1] or nil) or g;
+            if (rel.kind == "has_one") then
+                mains[i][name] = g[1];
+            else
+                mains[i][name] = g;
+            end
         end
         cb();
     end);
@@ -494,12 +502,14 @@ function NormOrm:_query_with_includes(model, state, includes, single)
             local records = {};
             for i = 1, #rows do records[i] = model:wrap(rows[i]); end
             if (#records == 0) then
-                return resolve(single and nil or records);
+                if (single) then return resolve(nil); end
+                return resolve(records);
             end
             local ok, perr = pcall(function()
                 self:_load_includes(model, records, includes, function(e)
                     if (e ~= nil) then return reject(e); end
-                    resolve(single and records[1] or records);
+                    if (single) then return resolve(records[1]); end
+                    resolve(records);
                 end);
             end);
             if (not ok) then reject(perr); end

tests/selftest.lua

@@ -1733,6 +1733,94 @@ fdb:define("t", { id = orm.types.id(), code = orm.types.string({ length = 8, ind
 local synced, sync_err = nil, nil;
 fdb:sync():next(function(v) synced = v; end, function(e) sync_err = e; end);
 check("a duplicate index does not fail sync on mysql", synced == true and sync_err == nil, tostring(sync_err));
+print("== Test group 41: boolean decoding ==");
+local bm = Mock({ dialect = "mysql" });
+local bdb = orm.new({ adapter = bm, promise = orm.promise.builtin() });
+local B = bdb:define("flags", { id = orm.types.id(), admin = orm.types.boolean() });
+
+bm.query_result = { { id = 1, admin = 0 } };
+local off = B:find(1):await();
+check("admin 0 decodes to boolean false", off.admin == false, tostring(off.admin));
+check("admin 0 is not truthy", not off.admin, type(off.admin));
+
+bm.query_result = { { id = 2, admin = 1 } };
+local on = B:find(2):await();
+check("admin 1 decodes to boolean true", on.admin == true, tostring(on.admin));
+
+bm.query_result = { { id = 3, admin = 0 } };
+local rel = B:find(3):await();
+rel:reload();
+check("reload keeps boolean false", rel.admin == false, tostring(rel.admin));
+print("== Test group 35: eager loading with nothing to attach ==");
+local em = Routed({ dialect = "mysql" });
+local edb = orm.new({ adapter = em, promise = orm.promise.builtin() });
+local EU = edb:define("eusers", {
+    id      = orm.types.id(),
+    posts   = orm.types.hasMany("eposts", { key = "user_id" }),
+    profile = orm.types.hasOne("eprofiles", { key = "user_id" }),
+});
+edb:define("eposts", { id = orm.types.id(), user_id = orm.types.integer() });
+edb:define("eprofiles", { id = orm.types.id(), user_id = orm.types.integer() });
+
+em.rows.eusers = {};
+local none = EU:query():include("posts"):first():await();
+check("first with include resolves nil when nothing matched", none == nil, tostring(none));
+
+em.rows.eusers = { { id = 1 }, { id = 2 } };
+em.rows.eposts = { { id = 9, user_id = 1 } };
+em.rows.eprofiles = {};
+local list = EU:query():include("posts", "profile"):all():await();
+check("parent with related rows gets them", #list[1].posts == 1, tostring(#list[1].posts));
+check("parent without related rows gets an empty list", #list[2].posts == 0);
+check("empty has_one is nil, not an empty table", list[1].profile == nil, type(list[1].profile));
+
+em.rows.eusers = { { id = 3 }, { id = 4 } };
+em.rows.eposts = {};
+local orphans = EU:query():include("posts"):all():await();
+table.insert(orphans[1].posts, "x");
+check("empty collections are not shared between parents", #orphans[2].posts == 0, tostring(#orphans[2].posts));
+print("== Test group 36: find_or_* honour the soft-delete scope ==");
+local fm = Mock({ dialect = "mysql" });
+local fdb2 = orm.new({ adapter = fm, promise = orm.promise.builtin() });
+local F = fdb2:define("faccounts", { id = orm.types.id(), email = orm.types.string({ length = 40 }) },
+    { soft_deletes = true });
+
+fm.query_result = {};
+F:find_or_new({ email = "a@b.c" }, { });
+local lookup;
+for _, c in ipairs(fm.calls) do
+    if (c.kind == "query" and c.sql:find("SELECT", 1, true)) then lookup = c.sql; end
+end
+check("find_or_new excludes trashed rows",
+    lookup:find("`email` = ? AND `deleted_at` IS NULL", 1, true) ~= nil, tostring(lookup));
+
+fm.calls = {};
+F:find_or_create({ email = "c@d.e" }, {});
+local lookup2;
+for _, c in ipairs(fm.calls) do
+    if (c.kind == "query" and c.sql:find("SELECT", 1, true)) then lookup2 = c.sql; break; end
+end
+check("find_or_create excludes trashed rows",
+    lookup2:find("`deleted_at` IS NULL", 1, true) ~= nil, tostring(lookup2));
+print("== Test group 37: upsert needs its conflict columns ==");
+local um = Mock({ dialect = "mysql" });
+local udb = orm.new({ adapter = um, promise = orm.promise.builtin() });
+local U3 = udb:define("uaccounts", {
+    id         = orm.types.id(),
+    account_id = orm.types.string({ length = 32, unique = true }),
+    name       = orm.types.string({ length = 20 }),
+});
+
+check("upsert without the default conflict column raises",
+    select(1, pcall(function() U3:upsert({ name = "Zoe" }); end)) == false);
+check("upsert with an explicit conflict column missing from the data raises",
+    select(1, pcall(function() U3:upsert({ name = "Zoe" }, { conflict = { "account_id" } }); end)) == false);
+
+um.query_result = { { id = 1, account_id = "acc-1", name = "Zoe" } };
+local rec = U3:upsert({ account_id = "acc-1", name = "Zoe" }, { conflict = { "account_id" } }):await();
+check("a valid upsert still reads the row back", rec ~= nil and rec.account_id == "acc-1", tostring(rec));
+check("the read-back filters on the conflict column",
+    last_sql(um):find("WHERE `account_id` = ?", 1, true) ~= nil, last_sql(um));
 end -- close the last group's scope
 
 print(("\n== RESULT: %d passed, %d failed =="):format(passed, failed));