diff options
author | Sean McGivern <sean@gitlab.com> | 2018-04-19 17:22:23 +0100 |
---|---|---|
committer | Sean McGivern <sean@gitlab.com> | 2018-06-05 14:57:19 +0100 |
commit | 6ecf819f73ae28547e1b816780de0d85c3a653cf (patch) | |
tree | f81c50bb4e8a1984530e130e61871173fc73d4b9 /app/uploaders | |
parent | e11a1001dcdc1ea5c65845fb0897b861b5c0b92d (diff) | |
download | gitlab-ce-6ecf819f73ae28547e1b816780de0d85c3a653cf.tar.gz |
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 'app/uploaders')
-rw-r--r-- | app/uploaders/object_storage.rb | 4 |
1 files changed, 2 insertions, 2 deletions
diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb index 3bb2e1ea63a..5aa1bc7227c 100644 --- a/app/uploaders/object_storage.rb +++ b/app/uploaders/object_storage.rb @@ -33,7 +33,7 @@ module ObjectStorage unless current_upload_satisfies?(paths, model) # the upload we already have isn't right, find the correct one - self.upload = uploads.find_by(model: model, path: paths) + self.upload = model&.retrieve_upload(identifier, paths) end super @@ -46,7 +46,7 @@ module ObjectStorage end def upload=(upload) - return unless upload + return if upload.nil? self.object_store = upload.store super |