Skip to content

Commit

Permalink
Merge pull request #1967 from MushroomObserver/nimmo-retry-matrix-box…
Browse files Browse the repository at this point in the history
…-cache-unicorn

Retry matrix box caching on unicorn, but restore checking `object_fragment_exist?`
  • Loading branch information
nimmolo committed Feb 27, 2024
2 parents 662f5d9 + 684d7e4 commit 38c45b4
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 24 deletions.
9 changes: 6 additions & 3 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ GEM
actionview (>= 5.0.0)
activesupport (>= 5.0.0)
json (2.7.1)
kgio (2.11.4)
language_server-protocol (3.17.0.3)
libv8-node (18.16.0.0-arm64-darwin)
libv8-node (18.16.0.0-x86_64-darwin)
Expand Down Expand Up @@ -195,8 +196,6 @@ GEM
psych (4.0.6)
stringio
public_suffix (5.0.4)
puma (6.4.2)
nio4r (~> 2.0)
racc (1.7.3)
rack (2.2.8)
rack-session (1.0.2)
Expand Down Expand Up @@ -226,6 +225,7 @@ GEM
thor (~> 1.0, >= 1.2.2)
zeitwerk (~> 2.6)
rainbow (3.1.1)
raindrops (0.20.1)
rake (13.1.0)
rbtree (0.4.6)
rdoc (6.6.2)
Expand Down Expand Up @@ -306,6 +306,9 @@ GEM
tzinfo (2.0.6)
concurrent-ruby (~> 1.0)
unicode-display_width (2.5.0)
unicorn (6.1.0)
kgio (~> 2.6)
raindrops (~> 0.7)
uniform_notifier (1.16.0)
web-console (4.2.1)
actionview (>= 6.0.0)
Expand Down Expand Up @@ -380,7 +383,6 @@ DEPENDENCIES
newrelic_rpm
nokogiri (>= 1.13.10)
psych (~> 4)
puma
rack (~> 2)
rails-controller-testing
rails-html-sanitizer (>= 1.4.4)
Expand All @@ -401,6 +403,7 @@ DEPENDENCIES
stimulus-rails
terser
turbo-rails
unicorn
web-console
webmock
xmlrpc
Expand Down
23 changes: 15 additions & 8 deletions app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1349,6 +1349,7 @@ def index
# id:: Warp to page that includes object with this id.
# action:: Template used to render results.
# matrix:: Displaying results as matrix?
# cache:: Cache the HTML of the results?
# letters:: Paginating by letter?
# letter_arg:: Param used to store letter for pagination.
# number_arg:: Param used to store page number for pagination.
Expand Down Expand Up @@ -1470,6 +1471,11 @@ def skip_if_coming_back(query, args)
end
end

# NOTE: there are two places where cache args have to be sent to enable
# efficient caching. Sending `cache: true` here to `show_index_of_objects`
# allows us to optimize eager-loading, doing it only for records not cached.
# (The other place is from the template to the `matrix_box` helper, which
# actually caches the HTML.)
def find_objects(query, args)
caching = args[:cache] || false
include = args[:include] || nil
Expand All @@ -1491,16 +1497,17 @@ def find_objects(query, args)

# If caching, only uncached objects need to eager_load the includes
def objects_with_only_needed_eager_loads(query, include)
# Not currently caching on user.
# user = User.current ? "logged_in" : "no_user"
# locale = I18n.locale
locale = I18n.locale
objects_simple = query.paginate(@pages)

# Temporarily disabling cached matrix boxes: eager load everything
ids_to_eager_load = objects_simple
# If temporarily disabling cached matrix boxes: eager load everything
# ids_to_eager_load = objects_simple

# ids_to_eager_load = objects_simple.reject do |obj|
# object_fragment_exist?(obj, user, locale)
# end.pluck(:id)
ids_to_eager_load = objects_simple.reject do |obj|
object_fragment_exist?(obj, locale)
end.pluck(:id)
# now get the heavy loaded instances:
objects_eager = query.model.where(id: ids_to_eager_load).includes(include)
# our Array extension: collates new instances with old, in original order
Expand All @@ -1510,11 +1517,11 @@ def objects_with_only_needed_eager_loads(query, include)
# Check if a cached partial exists for this object.
# digest_path_from_template from ActionView::Helpers::CacheHelper :nodoc:
# https://stackoverflow.com/a/77862353/3357635
def object_fragment_exist?(obj, user, locale)
def object_fragment_exist?(obj, locale)
template = lookup_context.find(action_name, lookup_context.prefixes)
digest_path = helpers.digest_path_from_template(template)

fragment_exist?([digest_path, obj, user, locale])
fragment_exist?([digest_path, obj, locale])
end

def show_index_render(args)
Expand Down
18 changes: 9 additions & 9 deletions app/helpers/matrix_box_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,10 @@ def render_cached_matrix_boxes(objects, locals)
# matrix box has one version except langs.
# css hides image vote ui when body.no-user
objects.each do |object|
# cache(object) do
concat(render(partial: "shared/matrix_box",
locals: { object: object }.merge(locals)))
# end
cache(object) do
concat(render(partial: "shared/matrix_box",
locals: { object: object }.merge(locals)))
end
end
end

Expand Down Expand Up @@ -79,23 +79,23 @@ def matrix_box_image(image = nil, **args)
# end

# NOTE: object_id may be "no_ID" for logs of deleted records
def matrix_box_details(presenter, object, object_id, identify)
def matrix_box_details(presenter, object_id, identify)
tag.div(class: "panel-body rss-box-details") do
[
matrix_box_what(presenter, object, object_id, identify),
matrix_box_what(presenter, object_id, identify),
matrix_box_where(presenter),
matrix_box_when_who(presenter)
].safe_join
end
end

def matrix_box_what(presenter, object, object_id, identify)
def matrix_box_what(presenter, object_id, identify)
# heading style: bigger if no image.
# TODO: make box layouts specific to object type
h_style = presenter.image_data ? "h5" : "h3"
what = presenter.what
what = presenter.what # for an obs or rss_log, it's the obs
consensus = presenter.consensus || nil
identify_ui = matrix_box_vote_or_propose_ui(identify, object, consensus)
identify_ui = matrix_box_vote_or_propose_ui(identify, what, consensus)

tag.div(class: "rss-what") do
[
Expand Down
5 changes: 1 addition & 4 deletions app/views/controllers/shared/_matrix_box.erb
Original file line number Diff line number Diff line change
Expand Up @@ -18,18 +18,15 @@ if presenter
else
image_args = {}
end
# interactive_image votes UI is per-user, so cache must be per-user
# "Helper" comment below is necessary because the helper calls `cache`
%>
<%= matrix_box(columns: columns, id: object_id) do
tag.div(class: "panel panel-default") do
[
tag.div(class: "panel-sizing") do
[
# Helper Dependency Updated: Jan 14, 2024 at 3am
matrix_box_image(image, **image_args),
matrix_box_details(presenter, object, object_id, identify),
matrix_box_details(presenter, object_id, identify),
].safe_join
end,
matrix_box_log_footer(presenter),
Expand Down

0 comments on commit 38c45b4

Please sign in to comment.