Resolve N+1 on Attachments in Search Results

[mbarnett]

Context

With the switch from Fedora to PostgreSQL, we're increasing pressure on the DB server, and consequently turning up some old inefficiencies that can now be addressed.

Previously, search results involved making:

1. one Solr query containing all of the data needed to render the results minus information about the attachments
2. 10 queries to Solr to fetch the attachments for the 10 results on the page.

Now, we are making:

1. one Solr query, giving us the IDs of the resulting model instances
2. 10 queries for the model instances
3. 10*N more queries for the model instances' attachments, where N is the number of attachments each instance has

This is excessive.

This PR aims to address point 3 (point 2 may be addressed separately at some point as per

jupiter/app/models/jupiter_core/solr_services/deferred_faceted_solr_query.rb

Line 119 in 291ebcc

# TODO: This is inefficient and we should look at batching up IDs

but note the significant challenge here is that not all Solr results will necessarily come from the same DB table, so batching it up into a single query is likely impossible unless we unify Item and Thesis)

A complicating factor here is that the ActiveStorage API names the eager-load scope according to the attached model so, for instance, Item(and Thesis), which has_many_attached :files, has an eager-loading scope of with_attached_files, whereas Community, which has_one_attached :logo, has an eager-loading scope of with_attached_logo. And Collection has no eager-loading scope at all, because it lacks any attachments.

DeferredFacetedSolrQuery potentially deals with results containing all of the above models at runtime, with no knowledge of which scope to call when making a query for the record in the result:

jupiter/app/models/jupiter_core/solr_services/deferred_faceted_solr_query.rb

Lines 120 to 121 in 291ebcc

	arclass = res['has_model_ssim'].first.sub(/^Ar/, '').constantize
	arclass.find(res['id'])

What's New

We introduce two new Class methods in the Depositable superclass (which all Solr-searchable models must inherit from).

1. A protected method, self.eager_attachment_scope, which should return a model-specific call to the correct scope for the kind of attachment specific to the model (so eg. Item returns self. with_attached_files). By default this method blows up noisily when called, to force us to remember to override it correctly in all subclasses. Models like Collection simply make this method a no-op by choosing to return self.

2. A public method, self.with_eagerly_loaded_attachments, which simply gives the models' client code a nicer DSLy method for accessing the scope.

DeferredFacetedSolrQuery is then altered to use the scope on all classes during reification of search results.

(unrelatedly, I also shifted the methods around in Item and Thesis to group all instance methods and class methods together, because they were kind of a random mess)

[ualbertalib-bot]

	1 Warning
⚠️	There are code changes, but no corresponding tests. Please include tests if this PR introduces any modifications in behavior.

Generated by 🚫 Danger

[murny]

Looks good to me 👍. Nice work.

Might be nice to use bullet gem (https://github.com/flyerhzm/bullet) to help us discover these in the future since we now on ActiveRecord 🤔

[mbarnett]

Good idea! I'll take that for a spin locally and try it out.