fix(ContentManager): look up existing row before insertOrUpdate - #262
Conversation
kyteinsky
left a comment
kyteinsky
left a comment
There was a problem hiding this comment.
thanks for the fix!
I had assumed it would auto-fallback on update when unique constrain fails but well it seems each db has its own way of doing that so your solution seems like the best approach here even with the two query calls instead of one.
|
A few other minor details:
|
insertOrUpdate() can only fall back to update() correctly when the entity's id is already known ahead of time. submitContent() always built a fresh QueueContentItem (no id), so re-submitting content that was already indexed (unique constraint on app_id/provider_id/item_id) threw InvalidArgumentException: Entity which should be updated has no id, and the update silently never happened (caught and logged). Look up the existing row by its unique key first and reuse its id when present, calling insert()/update() explicitly instead of relying on insertOrUpdate()'s exception-driven fallback. Fixes nextcloud#261 Signed-off-by: Olivier <oboeglen@users.noreply.github.com>
a038031 to
5196d43
Compare
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Signed-off-by: kyteinsky <kyteinsky@gmail.com> Assisted-by: Github Copilot:claude-opus-5
d674b5b to
27a789c
Compare
|
thanks again, I fixed the DB query to only fetch the |
2 participants
Fixes #261.
ContentManager::submitContent()always built a freshQueueContentItementity (no id) and called$this->mapper->insertOrUpdate($dbItem).QBMapper::insertOrUpdate()can only fall back toupdate()correctly when the entity's id is already known ahead of time — here it never is, so re-submitting content that was already indexed (unique constraint onapp_id/provider_id/item_id) threw:caught and logged by the existing
catch (Exception $e), meaning the update silently never happened.This PR looks up the existing row by its unique key first (new
QueueContentItemMapper::findByUniqueKey()) and reuses its id when found, callinginsert()/update()explicitly instead of relying oninsertOrUpdate()'s exception-driven fallback with a freshly-constructed entity.Reproduced with the
bookmarksapp re-crawling and re-submitting an already-indexed bookmark (full trace in #261), but the bug is generic to any provider re-submitting previously-indexed content.