Skip to content

Commit

Permalink
Fixes columnNameMappers applied to json column when using FieldExpres…
Browse files Browse the repository at this point in the history
…sions (Vincit#1089)
  • Loading branch information
cesumilo committed Mar 15, 2024
1 parent 43d0b9c commit e5dc99c
Show file tree
Hide file tree
Showing 19 changed files with 335 additions and 83 deletions.
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ profiling
npm-debug.log*
.vscode
/**/yarn.lock
/**/.vuepress/dist
/**/.vuepress/dist
docker-compose.override.yml
2 changes: 1 addition & 1 deletion doc/release-notes/changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@

### What's new

- Only add AJV formats if they weren't added in user-land already [#2482](https://github.com/Vincit/objection.js/pull/2482)
- Only add Ajv formats if they weren't added in user-land already [#2482](https://github.com/Vincit/objection.js/pull/2482)

## 3.1.0

Expand Down
30 changes: 20 additions & 10 deletions lib/model/AjvValidator.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,10 @@ class AjvValidator extends Validator {
self.cache = new Map();

const setupAjv = (ajv) => {
conf.onCreateAjv(ajv);
// Only add AJV formats if they weren't added in user-space already
if (conf.onCreateAjv) {
conf.onCreateAjv(ajv);
}
// Only add Ajv formats if they weren't added in user-space already
if (!ajv.formats['date-time']) {
addFormats(ajv);
}
Expand Down Expand Up @@ -77,7 +79,7 @@ class AjvValidator extends Validator {
}

validator.call(model, json);
const error = parseValidationError(validator.errors, modelClass, options);
const error = parseValidationError(validator.errors, modelClass, options, this.ajvOptions);

if (error) {
throw error;
Expand All @@ -87,7 +89,7 @@ class AjvValidator extends Validator {
}

getValidator(modelClass, jsonSchema, isPatchObject) {
// Use the AJV custom serializer if provided.
// Use the Ajv custom serializer if provided.
const createCacheKey = this.ajvOptions.serialize || JSON.stringify;

// Optimization for the common case where jsonSchema is never modified.
Expand All @@ -103,7 +105,7 @@ class AjvValidator extends Validator {
if (!validators) {
validators = {
// Validator created for the schema object without `required` properties
// using the AJV instance that doesn't set default values.
// using the Ajv instance that doesn't set default values.
patchValidator: null,

// Validator created for the unmodified schema.
Expand Down Expand Up @@ -143,7 +145,7 @@ class AjvValidator extends Validator {
}
}

function parseValidationError(errors, modelClass, options) {
function parseValidationError(errors, modelClass, options, ajvOptions) {
if (!errors) {
return null;
}
Expand Down Expand Up @@ -178,13 +180,21 @@ function parseValidationError(errors, modelClass, options) {
// More than one error can occur for the same key in Ajv, merge them in the array:
const array = errorHash[key] || (errorHash[key] = []);

// Use unshift instead of push so that the last error ends up at [0],
// preserving previous behavior where only the last error was stored.
array.unshift({
// Prepare error object
const errorObj = {
message: error.message,
keyword: error.keyword,
params: error.params,
});
};

// Add data if verbose enabled
if (ajvOptions.verbose) {
errorObj.data = error.data;
}

// Use unshift instead of push so that the last error ends up at [0],
// preserving previous behavior where only the last error was stored.
array.unshift(errorObj);

++numErrors;
}
Expand Down
26 changes: 21 additions & 5 deletions lib/model/Model.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,19 +98,26 @@ class Model {
const columnNameMappers = this.constructor.getColumnNameMappers();

if (columnNameMappers) {
json = columnNameMappers.parse(json);
json = columnNameMappers.parse(json, this.isJsonColumn(this.constructor.$$columnInfo));
}

return parseJsonAttributes(json, this.constructor);
}

isJsonColumn(columnInfo) {
return (columnName) =>
typeof columnInfo == 'object' &&
columnInfo.hasOwnProperty(columnName) &&
(columnInfo[columnName]['type'] == 'json' || columnInfo[columnName]['type'] == 'jsonb');
}

$formatDatabaseJson(json) {
const columnNameMappers = this.constructor.getColumnNameMappers();

json = formatJsonAttributes(json, this.constructor);

if (columnNameMappers) {
json = columnNameMappers.format(json);
json = columnNameMappers.format(json, this.isJsonColumn(this.constructor.$$columnInfo));
}

return json;
Expand Down Expand Up @@ -317,9 +324,6 @@ class Model {

static createValidator() {
return new AjvValidator({
onCreateAjv: (ajv) => {
/* Do Nothing by default */
},
options: {
allErrors: true,
validateSchema: false,
Expand Down Expand Up @@ -408,6 +412,10 @@ class Model {
return this.modifiers || {};
}

static async columnInfo(creator) {
return await cachedGetAsync(this, '$$columnInfo', creator);
}

static columnNameToPropertyName(columnName) {
let colToProp = cachedGet(this, '$$colToProp', () => new Map());
let propertyName = colToProp.get(columnName);
Expand Down Expand Up @@ -819,6 +827,14 @@ function relatedQuery({ modelClass, relationName, transaction, alwaysReturnArray
});
}

async function cachedGetAsync(target, hiddenPropertyName, creator) {
if (!target.hasOwnProperty(hiddenPropertyName)) {
defineNonEnumerableProperty(target, hiddenPropertyName, await creator(target));
}

return target[hiddenPropertyName];
}

function cachedGet(target, hiddenPropertyName, creator) {
if (!target.hasOwnProperty(hiddenPropertyName)) {
defineNonEnumerableProperty(target, hiddenPropertyName, creator(target));
Expand Down
1 change: 1 addition & 0 deletions lib/model/modelUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const staticHiddenProps = [
'$$readOnlyAttributes',
'$$idRelationProperty',
'$$virtualAttributes',
'$$columnInfo',
];

function defineNonEnumerableProperty(obj, prop, value) {
Expand Down
4 changes: 3 additions & 1 deletion lib/queryBuilder/QueryBuilder.js
Original file line number Diff line number Diff line change
Expand Up @@ -440,8 +440,11 @@ class QueryBuilder extends QueryBuilderBase {
async execute() {
// Take a clone so that we don't modify this instance during execution.
const builder = this.clone();
const colBuilder = this.clone();

try {
// Lazy fetch columns info (fixes #1089)
await builder._modelClass.columnInfo(async (_) => await colBuilder.columnInfo());
await beforeExecute(builder);
const result = await doExecute(builder);
return await afterExecute(builder, result);
Expand Down Expand Up @@ -1097,7 +1100,6 @@ function doExecute(builder) {
promise = Promise.resolve(queryExecutorOperation.queryExecutor(builder));
} else {
promise = Promise.resolve(buildKnexQuery(builder));

promise = chainOperationHooks(promise, builder, 'onRawResult');
promise = promise.then((result) => createModels(result, builder));
}
Expand Down
14 changes: 12 additions & 2 deletions lib/utils/identifierMapping.js
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ function mapLastPart(mapper, separator) {
// Returns a function that takes an object as an input and maps the object's keys
// using `mapper`. If the input is not an object, the input is returned unchanged.
function keyMapper(mapper) {
return (obj) => {
return (obj, isJsonColumn = () => false) => {
if (!isObject(obj) || Array.isArray(obj)) {
return obj;
}
Expand All @@ -155,7 +155,17 @@ function keyMapper(mapper) {

for (let i = 0, l = keys.length; i < l; ++i) {
const key = keys[i];
out[mapper(key)] = obj[key];

if (key.indexOf(':') > -1) {
const parts = key.split(':');
if (isJsonColumn(mapper(parts[0]))) {
out[`${mapper(parts[0])}:${parts[1]}`] = obj[key];
} else {
out[mapper(key)] = obj[key];
}
} else {
out[mapper(key)] = obj[key];
}
}

return out;
Expand Down
88 changes: 44 additions & 44 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit e5dc99c

Please sign in to comment.