Skip to content

Commit

Permalink
fix: dispatched result should be in attribute names (#198)
Browse files Browse the repository at this point in the history 
* fix: dispatched result should be in attribute names

... rather than the column names returned from database. also refactored src/collection.js a bit, some logic can be simplified by isolating select column aliasing in SQLite.

* refactor: drop driver.cast() and driver.uncast()

... replaced with attribute.cast() and attribute.uncast(), which carries attribute type by itself

* fix: Model.select(raw()) should try instantiate as well

* fix: date.uncast() should handle several non-standard date formats

* fix: grouped query results should never be instantiated

* fix: select('DISTINCT foo')
  • Loading branch information
@cyjake
cyjake authored Oct 18, 2021
1 parent 316925a commit d38f45c
Show file tree
Hide file tree
Showing 15 changed files with 287 additions and 302 deletions.
35 changes: 29 additions & 6 deletions src/bone.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1009,10 +1009,33 @@ class Bone {
return this.driver && this.driver.pool;
}

/**
* Get the attribute name of column, or translate the whole object
* @private
* @param {string|Record<string, Literal>} name
* @return {string|Record<string, Literal>}
*/
static alias(data) {
const { attributeMap } = this;

if (typeof data === 'string') {
const result = attributeMap[data];
return result ? result.name : data;
}

const result = {};
for (const key in data) {
const value = data[key];
const attribute = attributeMap[key];
result[attribute ? attribute.name : key] = value;
}
return result;
}

/**
* Get the column name from the attribute name
* @private
* @param {string} name
* @param {string} name
* @return {string}
*/
static unalias(name) {
Expand Down Expand Up @@ -1152,16 +1175,16 @@ class Bone {
* @return {Bone}
*/
static instantiate(row) {
const { attributes, driver, attributeMap } = this;
const { attributes, attributeMap } = this;
const instance = new this();

for (const columnName in row) {
const value = row[columnName];
if (attributeMap.hasOwnProperty(columnName)) {
const { jsType, name } = attributeMap[columnName];
const attribute = attributeMap[columnName];
if (attribute) {
// to make sure raw and rawSaved hold two different objects
instance._setRaw(name, driver.cast(value, jsType));
instance._setRawSaved(name, driver.cast(value, jsType));
instance._setRaw(attribute.name, attribute.cast(value));
instance._setRawSaved(attribute.name, attribute.cast(value));
} else {
if (!isNaN(value)) instance[columnName] = Number(value);
else if (!isNaN(Date.parse(value))) instance[columnName] = new Date(value);
Expand Down
131 changes: 31 additions & 100 deletions src/collection.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,87 +52,22 @@ class Collection extends Array {
}

/**
* get all columns
* @param {Spell} spell
* @returns {Array<String>} columns
*/
function getColumns(spell) {
const { joins, columns, Model } = spell;
const { tableAlias, table, driver } = Model;
if (!columns.length) return [];
let targetColumns = [];
const joined = Object.keys(joins).length > 0;
if (driver.type === 'sqlite' && joined) {
/**
* { type: 'alias', value: 'posts:id', args: [ { type: 'id', qualifiers: [ 'posts' ], value: 'ss' } ] },
*/
if (columns.length) {
for (const column of columns) {
targetColumns.push(...column.args.filter(
c => c.qualifiers && (c.qualifiers.includes(table) || c.qualifiers.includes(tableAlias))
).map(c => c.value));
}
}
} else {
targetColumns = columns.filter(c => !c.qualifiers || c.qualifiers.includes(table) || c.qualifiers.includes(tableAlias)).map(v => v.value);
}
return targetColumns;
}

/**
* join target instantiatable or not
* @param {Object} join
* @returns {boolean}
*/
function joinedInstantiatable(join) {
const { on, Model } = join;
const { tableAlias, table } = Model;
let columns = [];
if (on && on.args && on.args.length) {
for (const arg of on.args) {
const { type } = arg;
// { type: 'op', ..., args: [{ value: '', qualifiers: [] }] }
if (type === 'op' && arg.args && arg.args.length) {
columns.push(...arg.args.filter(
c => c.qualifiers && (c.qualifiers.includes(table) || c.qualifiers.includes(tableAlias))
).map(c => c.value));
} else if (arg.value && arg.qualifiers && (arg.qualifiers.includes(table) || arg.qualifiers.includes(tableAlias))) {
// { type: 'id', value: '', qualifiers: [] }
columns.push(arg.value);
}
}
}
if (!columns.length) return true;
const attributeKeys = Object.keys(Model.attributes);
return columns.some(r => attributeKeys.includes(r));
}

/**
* @param {Spell} spell
* @returns duplicate main Model
*/
function shouldFindJoinTarget(spell) {
const { Model } = spell;
const { primaryKey } = Model;
const columns = getColumns(spell);
return !columns.length || columns.length && columns.includes(primaryKey);
}

/**
* Check if the query result is instantiatable
* Check if the query result of spell is instantiatable by examining the query structure.
* - https://www.yuque.com/leoric/blog/ciiard#XoI4O (zh-CN)
* @param {Spell} spell
* @returns {boolean}
*/
function instantiatable(spell) {
const { Model, groups } = spell;
const { attributes } = Model;
const columns = getColumns(spell);
const { columns, groups, Model } = spell;
const { attributes, tableAlias } = Model;

if (groups.length > 0) return false;
if (!columns.length) return true;
const attributeKeys = Object.keys(attributes);
return columns.some(r => attributeKeys.includes(r));
}
if (columns.length === 0) return true;

return columns
.filter(({ qualifiers }) => (!qualifiers || qualifiers.includes(tableAlias)))
.every(({ value }) => attributes[value]);
}

/**
* Convert the results to collection that consists of models with their associations set up according to `spell.joins`.
Expand All @@ -144,7 +79,7 @@ function instantiatable(spell) {
*/
function dispatch(spell, rows, fields) {
const { groups, joins, columns, Model } = spell;
const { tableAlias, table, primaryKey, primaryColumn, attributes } = Model;
const { tableAlias, table, primaryKey, primaryColumn, attributeMap } = Model;

// await Post.count()
if (rows.length <= 1 && columns.length === 1 && groups.length === 0) {
Expand All @@ -157,9 +92,7 @@ function dispatch(spell, rows, fields) {
}

const joined = Object.keys(joins).length > 0;
const shouldFindDuplicate = shouldFindJoinTarget(spell);
const canInstantiate = instantiatable(spell);
const attributeKeys = Object.keys(attributes);

const results = new Collection();
for (const row of rows) {
Expand All @@ -174,12 +107,13 @@ function dispatch(spell, rows, fields) {
}
}
let current;
if (shouldFindDuplicate && result[primaryColumn] != null) {
if (joined && result[primaryColumn] != null) {
current = results.find(r => r[primaryKey] == result[primaryColumn]);
}
if (!current) {
const resultKeys = Object.keys(result);
current = canInstantiate || (!groups.length && resultKeys.some(c => attributeKeys.includes(c)))? Model.instantiate(result) : result;
current = canInstantiate || (groups.length === 0 && Object.keys(result).every(key => attributeMap[key]))
? Model.instantiate(result)
: Model.alias(result);
results.push(current);
}
if (joined) {
Expand All @@ -191,32 +125,29 @@ function dispatch(spell, rows, fields) {
}

function dispatchJoins(current, spell, row, fields) {
const instantiatableMap = {};
for (const qualifier in spell.joins) {
const join = spell.joins[qualifier];
const { Model, hasMany } = join;
if (instantiatableMap[qualifier] === undefined) instantiatableMap[qualifier] = joinedInstantiatable(join);
const joinInstantiatable = instantiatableMap[qualifier];
// mysql2 nests rows with table name instead of table alias.
const values = row[qualifier] || row[Model.table];
if (values) {
if (hasMany) {
const id = values[Model.primaryColumn];
if (!current[qualifier]) current[qualifier] = new Collection();
if (!Array.isArray(current[qualifier])) {
const origin = !(current[qualifier] instanceof Model) && joinInstantiatable? Model.instantiate(current[qualifier]) : current[qualifier];
current[qualifier] = new Collection();
if (Object.values(values).some(value => value != null)) {
current[qualifier].push(origin);
}

if (!values) continue;
if (hasMany) {
const id = values[Model.primaryColumn];
if (!current[qualifier]) current[qualifier] = new Collection();
if (!Array.isArray(current[qualifier])) {
const instance = Model.instantiate(current[qualifier]);
current[qualifier] = new Collection();
if (Object.values(values).some(value => value != null)) {
current[qualifier].push(instance);
}
if (!id || current[qualifier].some(item => item[Model.primaryKey] === id) || Object.values(values).every(value => value == null)) continue;
current[qualifier].push(joinInstantiatable? Model.instantiate(values) : values);
} else {
current[qualifier] = Object.values(values).some(value => value != null)
? (joinInstantiatable? Model.instantiate(values) : values)
: null;
}
if (!id || current[qualifier].some(item => item[Model.primaryKey] === id)) continue;
current[qualifier].push(Model.instantiate(values));
} else {
current[qualifier] = Object.values(values).some(value => value != null)
? Model.instantiate(values)
: null;
}
}
}
Expand Down
19 changes: 16 additions & 3 deletions src/data_types.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ class BIGINT extends INTEGER {
}
}

const rDateFormat = /^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}(?:[,.]\d{3,6})?$/;

class DATE extends DataType {
constructor(precision, timezone = true) {
super();
Expand All @@ -218,6 +220,12 @@ class DATE extends DataType {
return dataType;
}

cast(value) {
if (value == null) return value;
if (value instanceof Date) return value;
return new Date(value);
}

uncast(value) {
if (value == null) return value;
if (typeof value.toDate === 'function') {
Expand All @@ -231,6 +239,14 @@ class DATE extends DataType {
return result;
}

if (typeof value === 'string') {
// vaguely standard date formats such as 2021-10-15 15:50:02,548
if (rDateFormat.test(value)) {
return new Date(`${value.replace(' ', 'T').replace(',', '.')}Z`);
}
// Date.parse('2021-10-15T08:38:43.877Z')
return new Date(value);
}
return value instanceof Date ? value : new Date(value);
}
}
Expand Down Expand Up @@ -329,9 +345,6 @@ class JSON extends DataType {

uncast(value) {
if (value == null) return value;
if (typeof value.toObject === 'function') {
value = value.toObject();
}
return global.JSON.stringify(value);
}
}
Expand Down
63 changes: 0 additions & 63 deletions src/drivers/abstract/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,67 +15,4 @@ module.exports = class AbstractDriver {
this.idleTimeout = opts.idleTimeout || 60;
this.options = opts;
}

/**
* Cast raw values from database to JavaScript types. When the raw packet is fetched from database, `Date`s and special numbers are transformed by drivers already. This method is used to cast said values to custom types set by {@link Bone.attribute}, such as `JSON`.
* @private
* @param {string|boolean} value
* @param {Object|Date|string|Boolean} type
* @returns {Object|Date|string|boolean}
*/
cast(value, jsType) {
if (value == null) return value;

switch (jsType) {
case JSON:
if (!value) return null;
// type === JSONB
if (typeof value === 'object') return value;
return JSON.parse(value);
case Date:
return value instanceof Date ? value : new Date(value);
// node-sqlite3 doesn't convert TINYINT(1) to boolean by default
case Boolean:
return Boolean(value);
case Number:
// pg may return string
return Number(value);
case Buffer:
if (Buffer.isBuffer(value)) return value;
return Buffer.from(value);
default:
return value;
}
}

/**
* Uncast JavaScript values back to database types. This is the reverse version of {@link AbstractDriver#cast}.
* @private
* @param {Object|Date|string|boolean} value
* @param {Object|Date|String|Boolean} type
* @returns {boolean|number|string|Date}
*/
uncast(value, type) {
if (value == null) return value;
if (value != null && typeof value === 'object') {
if (type === JSON && typeof value.toObject === 'function') {
return JSON.stringify(value.toObject());
}
if (type === Date && typeof value.toDate === 'function') {
return value.toDate();
}
if (type === String && typeof value.toString === 'function') {
return value.toString();
}
}

switch (type) {
case JSON:
return JSON.stringify(value);
case Date:
return value instanceof Date ? value : new Date(value);
default:
return value;
}
}
};
5 changes: 4 additions & 1 deletion src/drivers/sqlite/spellbook.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,10 @@ module.exports = {
...spellbook,

formatSelect(spell) {
if (Object.keys(spell.joins).length > 0) renameSelectExpr(spell);
if (Object.keys(spell.joins).length > 0) {
spell = spell.dup;
renameSelectExpr(spell);
}
return spellbook.formatSelect(spell);
}
};
2 changes: 1 addition & 1 deletion src/expr_formatter.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ function formatExpr(spell, ast) {
case 'alias':
return `${formatExpr(spell, args[0])} AS ${formatIdentifier(spell, ast)}`;
case 'mod':
return `${name.to.toUpperCase()} ${formatExpr(spell, args[0])}`;
return `${name.toUpperCase()} ${formatExpr(spell, args[0])}`;
case 'id':
return formatIdentifier(spell, ast);
case 'op':
Expand Down
1 change: 0 additions & 1 deletion test/integration/suite/querying.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ describe('=> Query', function() {
Post.create({ id: 3, title: 'Archangel Tyrael', isPrivate: true }),
Post.create({ id: 4, title: 'Diablo', deletedAt: new Date(2012, 4, 15) })
]);

});

after(async function() {
Expand Down
Loading

0 comments on commit d38f45c

Please sign in to comment.