From 7dc0e3361a97aaeb76505eb7008d97a56dd1f443 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ju=CC=88rg=20Lehni?= Date: Thu, 15 Oct 2026 23:15:05 +0200 Subject: [PATCH] Name the formats of jsonColumnFormat after the columns, and resolve 'auto' by driver --- doc/api/model/static-properties.md | 9 +++--- doc/release-notes/changelog.md | 4 +-- doc/release-notes/migration.md | 15 +++++---- lib/model/modelJsonAttributes.js | 21 ++++++++---- tests/integration/misc/issue205.js | 52 ++++++++++++++++++++++++------ tests/ts/examples.ts | 10 ++++-- tests/unit/model/Model.js | 4 +-- typings/objection/index.d.ts | 2 +- 8 files changed, 84 insertions(+), 33 deletions(-) diff --git a/doc/api/model/static-properties.md b/doc/api/model/static-properties.md index fd83d1ad0..c2891e025 100644 --- a/doc/api/model/static-properties.md +++ b/doc/api/model/static-properties.md @@ -383,11 +383,12 @@ class Person extends Model { } ``` -How the database returns the columns of the [jsonAttributes](/api/model/static-properties.html#static-jsonattributes): +The kind of columns that hold the [jsonAttributes](/api/model/static-properties.html#static-jsonattributes). The format applies to all JSON attributes of the model, not per column: -- `'auto'` (default): the PostgreSQL drivers (`pg`, `pgnative`, `cockroachdb`) return `json` and `jsonb` columns parsed, and their values are left as they are. With other drivers, and in rows passed to `fromDatabaseJson()` by hand, strings that are valid JSON are parsed and other strings are kept. -- `'text'`: the columns hold JSON text, e.g. JSON in `text` columns on PostgreSQL, or on MariaDB. It is parsed with `JSON.parse()`, and invalid JSON throws an error. -- `'parsed'`: the driver returns the columns parsed. Their values are never parsed. +- `'auto'` (default): `'json'` on the PostgreSQL drivers (`pg`, `pgnative`, `cockroachdb`), `'text'` on SQLite and SQL Server, and `'mixed'` on MySQL and MariaDB, with other drivers and in rows passed to `fromDatabaseJson()` by hand. `mysql2` returns `json` columns parsed, so on MySQL, a string in them that looks like JSON, e.g. `'123'`, comes back parsed. Set `'json'` with `mysql2` on MySQL to keep such strings, but not with `mysql`, which returns JSON as text, and not on MariaDB, which stores it as text. +- `'json'`: `json` or `jsonb` columns, which the driver returns parsed. Their values are never parsed again. +- `'text'`: text columns that hold JSON, e.g. JSON in `text` columns on PostgreSQL, or on MariaDB. It is parsed with `JSON.parse()`, and invalid JSON throws an error. +- `'mixed'`: text columns that hold JSON or plain text. Strings that are valid JSON are parsed, and other strings are kept, so a string that looks like JSON, e.g. `'123'`, comes back parsed, also from `json` and `jsonb` columns on PostgreSQL. ## `static` cloneObjectAttributes diff --git a/doc/release-notes/changelog.md b/doc/release-notes/changelog.md index b264e8085..4aed4a7ee 100644 --- a/doc/release-notes/changelog.md +++ b/doc/release-notes/changelog.md @@ -34,7 +34,7 @@ - Fetching a relation for owner models whose join property holds an array, e.g. a JSON array of keys, throws an error instead of warning and finding no related models. `$relatedQuery()` keeps working for them. See [migration](/release-notes/migration.md#missing-and-array-join-properties-throw) [#114](https://github.com/ditojs/objection/issues/114) - `$setRelated()` and `$appendRelated()` keep the foreign keys in sync with the relation. For a `BelongsToOneRelation` the object's foreign key is set to the related model's key, e.g. `post.$setRelated('author', user)` sets `post.authorId`, or to `null` when the relation is cleared or the related model has no key. For `HasOneRelation` and `HasManyRelation` the related models' foreign keys are set to the object's key, or to `null` when the object has no key, and those of the models that are removed or replaced are set to `null` if they still point to the object. Conflicting foreign keys are overwritten. Many-to-many and has-one-through relations are unchanged, and `fromJson()`, `$setJson()` and queries don't sync keys. See [migration](/release-notes/migration.md#setrelated-and-appendrelated-sync-foreign-keys) [#2001](https://github.com/Vincit/objection.js/issues/2001) - `idColumn = undefined` means that the model has no primary key, like `idColumn = null`, instead of a column named `undefined` [#129](https://github.com/ditojs/objection/issues/129) -- On PostgreSQL drivers (`pg`, `pgnative`, `cockroachdb`), JSON attributes keep the values that the driver returns, so a `json` / `jsonb` column holding a string that looks like JSON, e.g. `'123'` or `'{"a":1}'`, is no longer parsed a second time, and JSON in `text` columns is no longer parsed. Set `jsonColumnFormat = 'text'` on models that keep JSON in `text` columns on PostgreSQL. Other drivers parse JSON attributes as in 3.x. See [migration](/release-notes/migration.md#json-in-postgresql-text-columns-is-no-longer-parsed) [#205](https://github.com/ditojs/objection/issues/205) +- On PostgreSQL drivers (`pg`, `pgnative`, `cockroachdb`), JSON attributes keep the values that the driver returns, so a `json` / `jsonb` column holding a string that looks like JSON, e.g. `'123'` or `'{"a":1}'`, is no longer parsed a second time, and JSON in `text` columns is no longer parsed. Set `jsonColumnFormat = 'text'` on models that keep JSON in `text` columns on PostgreSQL, or `'mixed'` for columns that mix JSON with plain text. On SQLite and SQL Server, JSON attributes are parsed strictly, and a string that isn't valid JSON throws an error where 3.x kept it; set `'mixed'` to keep it. MySQL drivers parse JSON attributes as in 3.x. See [migration](/release-notes/migration.md#json-in-postgresql-text-columns-is-no-longer-parsed) [#205](https://github.com/ditojs/objection/issues/205) - `snakeCaseMappers()` only converts the column part of field expressions like `patch({ 'jsonColumn:someKey': 1 })` and keeps their JSON keys as written: the `preserveJsonKeys` option defaults to `true`. Set `preserveJsonKeys: false` to convert the JSON keys to snake case too, as in 3.x. `knexSnakeCaseMappers()` never converted them. Keys with a `:` that the model's `jsonSchema` declares in `properties` are columns and converted as a whole, so a column like `ns:foo_bar` round-trips as `ns:fooBar`. See [migration](/release-notes/migration.md#snakecasemappers-keeps-the-json-keys-of-field-expressions) [#1089](https://github.com/Vincit/objection.js/issues/1089) - `insert()` and `update()` reject field expression keys like `'meta:color'` with a `ValidationError` of type `ModelValidation`, as they write whole values. 3.x `update()` writes them with `jsonb_set` and validates them like a patch, and 3.x `insert()` passes them to the database as column names. Use `patch()` to write parts of json columns. Keys with a `:` that the schema declares in `properties`, e.g. `'ns:name'`, are normal properties and stay allowed, as do keys that don't parse as field expressions, e.g. `'ns:'`, which `patch()` and `merge()` also write as columns instead of failing to parse them. Only patch validation validates field expression keys against the json column's schema: `fromJson()` without `patch` validates them as top-level properties, as before 3.8. See [migration](/release-notes/migration.md#insert-and-update-reject-field-expression-keys) [#160](https://github.com/ditojs/objection/issues/160) - JSON field expressions (`ref('column:path')`, `'column:path'` keys in `patch()`), `ref('column').castJson()` and the `whereJson*()` methods of objection throw an error on SQL Server and Oracle instead of warning, as they generate SQL that these databases don't understand, and so do `whereJsonSupersetOf()` and `whereJsonSubsetOf()` on SQLite. Other databases than PostgreSQL, MySQL and SQLite still get a warning. See [migration](/release-notes/migration.md#json-field-expressions-throw-on-sql-server-and-oracle) [#113](https://github.com/ditojs/objection/issues/113) @@ -77,7 +77,7 @@ - `insert()` of several models works on SQLite (3.35 or newer) through `returning`, and on all databases when all models have their ids or the model has no `idColumn`, e.g. join rows. Rows that don't fit into one query, or that leave out columns that other rows have on SQLite, are inserted in several queries, which are only atomic in a transaction. - `BelongsToManyRelation`, a relation where the owner holds an array of the related models' keys, e.g. `posts.tagIds = [1, 4, 7]` for `post.tags`, in a JSON column on all databases or a native array column on PostgreSQL. It supports finding the related models with `$relatedQuery()`, `relatedQuery()`, `withGraphFetched()`, `withGraphJoined()` and `joinRelated()`. `insert()`, `delete()`, `relate()` and `unrelate()` through the relation rewrite the arrays, `insertGraph()` and `upsertGraph()` write them, and `$setRelated()` and `$appendRelated()` keep them in sync. The reads of the keys that these writes run call no find hooks. See [relations](/guide/relations.md) [#2667](https://github.com/Vincit/objection.js/issues/2667) -- `Model.jsonColumnFormat` says how the database returns JSON columns: `'auto'` (default) leaves them as they are on PostgreSQL drivers and parses JSON text on others, `'text'` parses them strictly and throws for invalid JSON, `'parsed'` never parses them [#205](https://github.com/ditojs/objection/issues/205) +- `Model.jsonColumnFormat` says what kind of columns hold the JSON attributes: `'auto'` (default) is `'json'` on PostgreSQL drivers, `'text'` on SQLite and SQL Server, and `'mixed'` on MySQL and others, `'json'` never parses the values that the driver returns, `'text'` parses JSON text strictly and throws for invalid JSON, `'mixed'` parses the strings that are valid JSON and keeps the others [#205](https://github.com/ditojs/objection/issues/205) - JSON field expressions work on MySQL and SQLite: in `ref()`, e.g. `ref('column:a.b').castInt()` in `select()`, `where()`, `orderBy()`, joins and relation mappings, as keys in `patch()` and `merge()`, e.g. `patch({ 'column:a.b': 1 })`, and in the `whereJson*()` methods, except for `whereJsonSupersetOf()` and `whereJsonSubsetOf()` on SQLite. objection writes the SQL of each database, `JSON_EXTRACT()` and `JSON_SET()` on MySQL and `json_extract()` and `json_set()` on SQLite, and casts to their types. `ref().castJson()` works on both too. See [JSON queries](/recipes/json-queries.md) [#276](https://github.com/Vincit/objection.js/issues/276) - `asArray()` is exported, to turn the `result` of static hooks into an array [#66](https://github.com/ditojs/objection/issues/66) - The hooks of the queries that `relate()` and `unrelate()` run get the `relation` and its owners as `items`: the update hooks of the related model class for `HasManyRelation`, of the owner model class for `BelongsToOneRelation` and `BelongsToManyRelation` (which also runs find hooks), and the insert and delete hooks of the join model class for `ManyToManyRelation` [#66](https://github.com/ditojs/objection/issues/66) diff --git a/doc/release-notes/migration.md b/doc/release-notes/migration.md index e5b5e84af..7703f45cd 100644 --- a/doc/release-notes/migration.md +++ b/doc/release-notes/migration.md @@ -51,7 +51,7 @@ This section lists the breaking changes in objection 4.0. - [Misplaced `*` and `^` in relation expressions throw](#misplaced-and-in-relation-expressions-throw) - [`$setRelated` and `$appendRelated` sync foreign keys](#setrelated-and-appendrelated-sync-foreign-keys) - [findById() and findByIds() keep their id condition in its own group](#findbyid-and-findbyids-keep-their-id-condition-in-its-own-group) -- [JSON in PostgreSQL text columns is no longer parsed](#json-in-postgresql-text-columns-is-no-longer-parsed) +- [JSON attributes are parsed by column format](#json-in-postgresql-text-columns-is-no-longer-parsed) ## objection is an ES module package @@ -569,15 +569,16 @@ Queries that relied on this, like a lookup by id or slug, need to spell out both await Person.query().where('id', idOrSlug).orWhere('slug', idOrSlug).first(); ``` -## JSON in PostgreSQL text columns is no longer parsed +## JSON attributes are parsed by column format {#json-in-postgresql-text-columns-is-no-longer-parsed} -The PostgreSQL drivers return `json` and `jsonb` columns parsed. 3.x parsed every string that came out of them a second time, so a JSON attribute that holds the string `'123'` came back as the number `123`, and saving the model wrote the change back. 4.0 keeps the values that these drivers return, so JSON stored in `text` columns comes back as a string. Change these columns to `jsonb`: +The PostgreSQL drivers return `json` and `jsonb` columns parsed. 3.x parsed every string that came out of them a second time, so a JSON attribute that holds the string `'123'` came back as the number `123`, and saving the model wrote the change back. 4.0 keeps the values that these drivers return, so JSON stored in `text` columns comes back as a string. Change these columns to `jsonb`. On PostgreSQL 16 or newer, this also keeps the rows that hold plain text, as JSON strings: ```sql -ALTER TABLE persons ALTER COLUMN address TYPE jsonb USING address::jsonb; +ALTER TABLE persons ALTER COLUMN address TYPE jsonb + USING CASE WHEN address IS JSON THEN address::jsonb ELSE to_jsonb(address) END; ``` -Or tell objection that the model's JSON columns hold text, which it then parses strictly, throwing for invalid JSON: +This turns `'{"a":1}'`, `'plain text'`, `'123'` and `NULL` into `{"a":1}`, `"plain text"`, `123` and `null`, deciding once what each row holds. Where the column can't be changed, tell objection what kind of column it is: `'text'` for JSON text, which it parses strictly, throwing for invalid JSON, and `'mixed'` for JSON mixed with plain text, where it parses the strings that are valid JSON and keeps the others, as 3.x did: ```js class Person extends Model { @@ -585,7 +586,9 @@ class Person extends Model { } ``` -Other drivers parse JSON attributes as in 3.x. On MariaDB, which stores JSON as `LONGTEXT`, `jsonColumnFormat = 'text'` avoids guessing whether a string is JSON. +The format applies to all JSON attributes of a model, not per column. With `'mixed'`, a string that looks like JSON, e.g. `'123'`, comes back as the number `123`, also from the model's `json` and `jsonb` columns. + +On SQLite and SQL Server, which return JSON as text, JSON attributes are parsed strictly, so a string that isn't valid JSON throws `Invalid JSON in JSON attribute …`, where 3.x kept the string. Set `jsonColumnFormat = 'mixed'` on models whose JSON columns also hold plain text there. MySQL drivers parse JSON attributes as in 3.x, so with `mysql2`, which returns `json` columns parsed, a string in them that looks like JSON, e.g. `'123'`, comes back as the number `123`. Set `jsonColumnFormat = 'json'` with `mysql2` on MySQL to keep such strings, but not with `mysql`, which returns JSON as text, and not on MariaDB, which stores it as text. On MariaDB, which stores JSON as `LONGTEXT`, `jsonColumnFormat = 'text'` avoids guessing whether a string is JSON. Overrides of the static [fromDatabaseJson](/api/model/static-methods.html#static-fromdatabasejson), [\$setDatabaseJson](/api/model/instance-methods.html#setdatabasejson) and [\$parseDatabaseJson](/api/model/instance-methods.html#parsedatabasejson) get the query's `options` as second argument. Pass them on to `super`, or JSON attributes are parsed as in 3.x. diff --git a/lib/model/modelJsonAttributes.js b/lib/model/modelJsonAttributes.js index bd8e86591..886dd6965 100644 --- a/lib/model/modelJsonAttributes.js +++ b/lib/model/modelJsonAttributes.js @@ -1,12 +1,12 @@ import { asArray, isObject, flatten, isString } from '../utils/objectUtils.js'; import { isQueryProp } from './modelQueryProps.js'; -import { isJsonParsedByDriver } from '../utils/knexUtils.js'; +import { isJsonParsedByDriver, isMsSql, isSqlite } from '../utils/knexUtils.js'; export function parseJsonAttributes(json, modelClass, knex) { const jsonAttr = modelClass.getJsonAttributes(); const format = getJsonColumnFormat(modelClass, knex); - if (jsonAttr.length && format !== 'parsed') { + if (jsonAttr.length && format !== 'json') { // JSON attributes may be returned as strings depending on the database and // the database client. Convert them to objects here. for (let i = 0, l = jsonAttr.length; i < l; ++i) { @@ -85,14 +85,23 @@ function getPropertyTypes(prop) { return types; } -// Resolve 'auto' to 'parsed' for drivers that return JSON columns parsed, or -// else to `null`: JSON attributes may be text then, and are parsed leniently. +// Resolve 'auto' by the driver: the PostgreSQL drivers return JSON columns +// parsed, SQLite and SQL Server return JSON text, and MySQL's drivers and others +// may return JSON or plain text, e.g. from MariaDB, which stores JSON as text, +// and which knex can't tell apart from MySQL. Without a driver, rows passed by +// hand may hold either. function getJsonColumnFormat(modelClass, knex) { const { jsonColumnFormat } = modelClass; if (jsonColumnFormat === 'auto') { - return knex && isJsonParsedByDriver(knex) ? 'parsed' : null; - } else if (jsonColumnFormat === 'text' || jsonColumnFormat === 'parsed') { + return !knex + ? 'mixed' + : isJsonParsedByDriver(knex) + ? 'json' + : isSqlite(knex) || isMsSql(knex) + ? 'text' + : 'mixed'; + } else if (['json', 'text', 'mixed'].includes(jsonColumnFormat)) { return jsonColumnFormat; } else { throw new Error( diff --git a/tests/integration/misc/issue205.js b/tests/integration/misc/issue205.js index 8d82cf622..b47096b54 100644 --- a/tests/integration/misc/issue205.js +++ b/tests/integration/misc/issue205.js @@ -112,13 +112,47 @@ export default (session) => { expect(item.data).toEqual({ a: 1 }); }); - it('should keep text in JSON columns that is not JSON', async () => { - await knex('issue205Item').insert({ id: 1, data: 'not json' }); - const item = await Item.query().findById(1); - expect(item.data).toBe('not json'); - }); + // MySQL's columns may hold JSON or plain text, SQLite's hold JSON text: + if (session.isMySql()) { + it('should keep text in JSON columns that is not JSON', async () => { + await knex('issue205Item').insert({ id: 1, data: 'not json' }); + const item = await Item.query().findById(1); + expect(item.data).toBe('not json'); + }); + } else { + it('should throw for text in JSON columns that is not JSON', async () => { + await knex('issue205Item').insert({ id: 1, data: 'not json' }); + await expect(Item.query().findById(1)).rejects.toThrow( + "Invalid JSON in JSON attribute 'data' of model 'Item'", + ); + }); + } } + it("should parse JSON text and keep other text with jsonColumnFormat 'mixed'", async () => { + class MixedItem extends Item { + static jsonColumnFormat = 'mixed'; + } + + await knex('issue205Item').insert([ + { id: 1, data: '{"a":1}' }, + { id: 2, data: 'plain text' }, + ]); + expect((await MixedItem.query().findById(1)).data).toEqual({ a: 1 }); + expect((await MixedItem.query().findById(2)).data).toBe('plain text'); + }); + + it("should read the same with jsonColumnFormat 'auto' as by default", async () => { + class AutoItem extends Item { + static jsonColumnFormat = 'auto'; + } + + await knex('issue205Item').insert({ id: 1, meta: JSON.stringify('123'), data: '{"a":1}' }); + expect((await AutoItem.query().findById(1)).toJSON()).toEqual( + (await Item.query().findById(1)).toJSON(), + ); + }); + it("should parse JSON text strictly with jsonColumnFormat 'text'", async () => { class TextItem extends Item { static jsonColumnFormat = 'text'; @@ -134,9 +168,9 @@ export default (session) => { ); }); - it("should not parse JSON text with jsonColumnFormat 'parsed'", async () => { + it("should not parse JSON text with jsonColumnFormat 'json'", async () => { class ParsedItem extends Item { - static jsonColumnFormat = 'parsed'; + static jsonColumnFormat = 'json'; } await knex('issue205Item').insert({ id: 1, data: '{"a":1}' }); @@ -145,12 +179,12 @@ export default (session) => { it('should throw for an invalid jsonColumnFormat', async () => { class InvalidItem extends Item { - static jsonColumnFormat = 'json'; + static jsonColumnFormat = 'parsed'; } await knex('issue205Item').insert({ id: 1 }); await expect(InvalidItem.query().findById(1)).rejects.toThrow( - "Invalid jsonColumnFormat of model 'InvalidItem': 'json'.", + "Invalid jsonColumnFormat of model 'InvalidItem': 'parsed'.", ); }); diff --git a/tests/ts/examples.ts b/tests/ts/examples.ts index e17a63a5a..c6a884ef1 100644 --- a/tests/ts/examples.ts +++ b/tests/ts/examples.ts @@ -1442,7 +1442,11 @@ class TextJsonModel extends objection.Model { } class ParsedJsonModel extends objection.Model { - static jsonColumnFormat: objection.JsonColumnFormat = 'parsed'; + static jsonColumnFormat: objection.JsonColumnFormat = 'json'; +} + +class MixedJsonModel extends objection.Model { + static jsonColumnFormat = 'mixed' as const; } class DatabaseJsonModel extends objection.Model { @@ -1459,9 +1463,9 @@ class DatabaseJsonModel extends objection.Model { } } -// @ts-expect-error jsonColumnFormat only takes 'auto', 'text' and 'parsed' +// @ts-expect-error jsonColumnFormat only takes 'auto', 'json', 'text' and 'mixed' class InvalidJsonFormatModel extends objection.Model { - static jsonColumnFormat = 'json' as const; + static jsonColumnFormat = 'parsed' as const; } async () => { diff --git a/tests/unit/model/Model.js b/tests/unit/model/Model.js index 76d9b1cff..d76e48737 100644 --- a/tests/unit/model/Model.js +++ b/tests/unit/model/Model.js @@ -980,9 +980,9 @@ describe('Model', () => { ); }); - it("should not parse JSON attributes with jsonColumnFormat 'parsed'", () => { + it("should not parse JSON attributes with jsonColumnFormat 'json'", () => { Model1.jsonAttributes = ['a']; - Model1.jsonColumnFormat = 'parsed'; + Model1.jsonColumnFormat = 'json'; expect(Model1.fromDatabaseJson({ a: '{"d":1}' }).a).toBe('{"d":1}'); }); }); diff --git a/typings/objection/index.d.ts b/typings/objection/index.d.ts index 689c41209..e1dabfb20 100644 --- a/typings/objection/index.d.ts +++ b/typings/objection/index.d.ts @@ -2950,7 +2950,7 @@ export interface SnakeCaseMappersOptions extends SnakeCaseOptions { export interface KnexSnakeCaseMappersOptions extends SnakeCaseOptions {} -export type JsonColumnFormat = 'auto' | 'text' | 'parsed'; +export type JsonColumnFormat = 'auto' | 'json' | 'text' | 'mixed'; // Passed by queries to `fromDatabaseJson()`, `$setDatabaseJson()` and // `$parseDatabaseJson()`. Overrides pass them on to `super`.