objection.js: Can't add or modify a value during $beforeUpdate using upsertGraph
We are trying to push some of calculated logic into the models using instance hooks, there is a case of encryption and decryption as well. This was working correctly with objection 1 but started showing a different behavior with objection 3. The problem is that since objection is doing a diff right before $beforeUpdate is invoked, the values changed in the function don’t get picked up as a part of the propsToUpdate
and they are not executed in the sql query. I tried to bypass this or somehow tried to change this value. The only thing that work was iterating over $$omitFromDatabaseJson and making sure that the field I want updated is not in this array. It would be great to have an UpsertGraphOptions to skip the diff check. The use case for us is just to update a graph
The code relevant is here https://github.com/Vincit/objection.js/blob/260b284a1cbfb044991894c5a3cf3dedc8ce7267/lib/queryBuilder/graph/patch/GraphPatchAction.js#L60
let Model
try {
Model = require('./').Model
} catch (err) {
Model = require('objection').Model
}
const Knex = require('knex')
const chai = require('chai')
async function main () {
await createSchema()
const item = { multiplier: 1 }
// The random value will get modified in $beforeInsert
const returned = await Item.query().insertGraphAndFetch(item)
// The random value won't be persisted into the DB
const newGeneratedRandom = await Item.query().upsertGraphAndFetch({ ...returned, multiplier: 200 })
chai.expect(newGeneratedRandom.random).to.not.equal(returned.random)
}
/// ////////////////////////////////////////////////////////////
// Database
/// ////////////////////////////////////////////////////////////
const knex = Knex({
client: 'sqlite3',
useNullAsDefault: true,
debug: false,
connection: {
filename: ':memory:',
},
})
Model.knex(knex)
/// ////////////////////////////////////////////////////////////
// Models
/// ////////////////////////////////////////////////////////////
class Item extends Model {
static get tableName () {
return 'item'
}
$beforeInsert () {
this.random = Math.ceil(Math.random() * this.multiplier)
}
$beforeUpdate () {
this.random = Math.ceil(Math.random() * this.multiplier)
}
}
/// ////////////////////////////////////////////////////////////
// Schema
/// ////////////////////////////////////////////////////////////
async function createSchema () {
await knex.schema
.dropTableIfExists('item')
await knex.schema
.createTable('item', table => {
table.increments('id').primary()
table.integer('multiplier')
table.integer('random')
})
}
main()
.then(() => {
console.log('success')
return knex.destroy()
})
.catch(err => {
console.error(err)
return knex.destroy()
})
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 2
- Comments: 26 (1 by maintainers)
Commits related to this issue
- Support $beforeUpdate() mutations in upsertGraph() Closes #2233 — committed to Vincit/objection.js by lehni a year ago
- Support $beforeUpdate() mutations in upsertGraph() Closes #2233 — committed to Vincit/objection.js by lehni a year ago
- Support $beforeUpdate() mutations in upsertGraph() Closes #2233 — committed to Vincit/objection.js by lehni a year ago
- Support $beforeUpdate() mutations in upsertGraph() Closes #2233 — committed to Vincit/objection.js by lehni a year ago
- Support $beforeUpdate() mutations in upsertGraph() Closes #2233 — committed to Vincit/objection.js by lehni a year ago
- Support $beforeUpdate() mutations in upsertGraph() Closes #2233 — committed to Vincit/objection.js by lehni a year ago
- Support $beforeUpdate() mutations in upsertGraph() Closes #2233 — committed to Vincit/objection.js by lehni a year ago
Ran into the same issue upgrading from objection 1.x to 2.x
This is going to be extremely common as it follows a common pattern recommended by this repo https://github.com/Vincit/objection.js/blob/260b284a1cbfb044991894c5a3cf3dedc8ce7267/doc/recipes/timestamps.md There is a base class with timestamps, and other models extend that class. This doesn’t work with upsertGraphAndFetch! Should be fixed!
@lehni @umutuzgur I’ve just run our test suites with the changes from https://github.com/Vincit/objection.js/pull/2452 applied and nothing broke.
Good news: Not a single fail across all our tests, and we use graph upserts quite extensively
Thank you for this fix !!!
Released in v3.1.0
Tested it, everything works as expect 👏
So I was curious about how to solve this, and learn more bout Objection’s internals along the way. I found a way to do so in e946c13a85058145cdcb361bd5244574bdc7b177 using
runBefore()
, but it’s a bit of a hack, and also a breaking change, as you can see in the changed tests that now getbeforeUpdateCalled
increased before determining that there’s nothing to udpdate.I am not sure if this should be merged or not, but if we would do so, it would be
v3.1.0
.@falkenhawk do you have an opinion on this?