diff options
author | Sean McGivern <sean@gitlab.com> | 2018-04-19 19:22:23 +0300 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2018-06-05 16:57:19 +0300 |
commit | 6ecf819f73ae28547e1b816780de0d85c3a653cf (patch) | |
tree | f81c50bb4e8a1984530e130e61871173fc73d4b9 /spec/uploaders/workers | |
parent | e11a1001dcdc1ea5c65845fb0897b861b5c0b92d (diff) |
Fix an N+1 in avatar URLs
This is tricky: the query was being run in
`ObjectStorage::Extension::RecordsUploads#retrieve_from_store!`, but we can't
just add batch loading there, because the `#upload=` method there would use the
result immediately, making the batch only have one item.
Instead, we can pre-emptively add an item to the batch whenever an avatarable
object is initialized, and then reuse that batch item in
`#retrieve_from_store!`. However, this also has problems:
1. There is a lot of logic in `Avatarable#retrieve_upload_from_batch`.
2. Some of that logic constructs a 'fake' model for the batch key. This should
be fine, because of ActiveRecord's override of `#==`, but it relies on that
staying the same.
Diffstat (limited to 'spec/uploaders/workers')
-rw-r--r-- | spec/uploaders/workers/object_storage/background_move_worker_spec.rb | 6 |
1 files changed, 4 insertions, 2 deletions
diff --git a/spec/uploaders/workers/object_storage/background_move_worker_spec.rb b/spec/uploaders/workers/object_storage/background_move_worker_spec.rb index b34f427fd8a..95813d15e52 100644 --- a/spec/uploaders/workers/object_storage/background_move_worker_spec.rb +++ b/spec/uploaders/workers/object_storage/background_move_worker_spec.rb @@ -125,8 +125,10 @@ describe ObjectStorage::BackgroundMoveWorker do it "migrates file to remote storage" do perform + project.reload + BatchLoader::Executor.clear_current - expect(project.reload.avatar.file_storage?).to be_falsey + expect(project.avatar).not_to be_file_storage end end @@ -137,7 +139,7 @@ describe ObjectStorage::BackgroundMoveWorker do it "migrates file to remote storage" do perform - expect(project.reload.avatar.file_storage?).to be_falsey + expect(project.reload.avatar).not_to be_file_storage end end end |