Merge pull request #3933 from evazion/fix-tag-set-presenter

Optimize tag set presenters

app/logical/storage_manager.rb

@@ -115,7 +115,7 @@ class StorageManager
   def seo_tags(post)
     return "" if !tagged_filenames
 
-    tags = post.humanized_essential_tag_string.gsub(/[^a-z0-9]+/, "_").gsub(/(?:^_+)|(?:_+$)/, "").gsub(/_{2,}/, "_")
+    tags = post.presenter.humanized_essential_tag_string.gsub(/[^a-z0-9]+/, "_").gsub(/(?:^_+)|(?:_+$)/, "").gsub(/_{2,}/, "_")
     "__#{tags}__"
   end
 end

app/models/post.rb

@@ -36,7 +36,6 @@ class Post < ApplicationRecord
   after_save :create_version
   after_save :update_parent_on_save
   after_save :apply_post_metatags
-  after_save :expire_essential_tag_string_cache
   after_commit :delete_files, :on => :destroy
   after_commit :remove_iqdb_async, :on => :destroy
   after_commit :update_iqdb_async, :on => :create
@@ -936,36 +935,6 @@ class Post < ApplicationRecord
       end
     end
 
-    def expire_essential_tag_string_cache
-      Cache.delete("hets-#{id}")
-    end
-
-    def humanized_essential_tag_string
-      @humanized_essential_tag_string ||= Cache.get("hets-#{id}", 1.hour.to_i) do
-        string = []
-
-        TagCategory.humanized_list.each do |category|
-          typetags = typed_tags(category) - TagCategory.humanized_mapping[category]["exclusion"]
-          if TagCategory.humanized_mapping[category]["slice"] > 0
-            typetags = typetags.slice(0,TagCategory.humanized_mapping[category]["slice"]) + (typetags.length > TagCategory.humanized_mapping[category]["slice"] ? ["others"] : [])
-          end
-          if TagCategory.humanized_mapping[category]["regexmap"] != //
-            typetags = typetags.map do |tag|
-              tag.match(TagCategory.humanized_mapping[category]["regexmap"])[1]
-            end
-          end
-          if typetags.any?
-            if category != "copyright" || typed_tags("character").any?
-              string << TagCategory.humanized_mapping[category]["formatstr"] % typetags.to_sentence
-            else
-              string << typetags.to_sentence
-            end
-          end
-        end
-        string.empty? ? "##{id}" : string.join(" ").tr("_", " ")
-      end
-    end
-
     TagCategory.categories.each do |category|
       define_method("tag_string_#{category}") do
         typed_tags(category).join(" ")

app/models/tag.rb

@@ -56,10 +56,6 @@ class Tag < ApplicationRecord
     extend ActiveSupport::Concern
 
     module ClassMethods
-      def counts_for(tag_names)
-        select_all_sql("SELECT name, post_count FROM tags WHERE name IN (?)", tag_names)
-      end
-
       def highest_post_count
         Cache.get("highest-post-count", 4.hours) do
           select("post_count").order("post_count DESC").first.post_count

app/presenters/post_presenter.rb

@@ -1,6 +1,6 @@
 class PostPresenter < Presenter
   attr_reader :pool, :next_post_in_pool
-  delegate :tag_list_html, :split_tag_list_html, :inline_tag_list_html, to: :tag_set_presenter
+  delegate :tag_list_html, :split_tag_list_html, :split_tag_list_text, :inline_tag_list_html, to: :tag_set_presenter
 
   def self.preview(post, options = {})
     if post.nil?
@@ -144,29 +144,13 @@ class PostPresenter < Presenter
   end
 
   def humanized_essential_tag_string
-    @post.humanized_essential_tag_string
+    @humanized_essential_tag_string ||= tag_set_presenter.humanized_essential_tag_string(default: "##{@post.id}")
   end
 
   def filename_for_download
     "#{humanized_essential_tag_string} - #{@post.md5}.#{@post.file_ext}"
   end
 
-  def categorized_tag_groups
-    string = []
-
-    TagCategory.categorized_list.each do |category|
-      if @post.typed_tags(category).any?
-        string << @post.typed_tags(category).join(" ")
-      end
-    end
-    
-    string
-  end
-
-  def categorized_tag_string
-    categorized_tag_groups.join(" \n")
-  end
-
   def safe_mode_message(template)
     html = ["This image is unavailable on safe mode (#{Danbooru.config.app_name}). Go to "]
     html << template.link_to("Danbooru", "https://danbooru.donmai.us") # XXX don't hardcode.

app/presenters/post_set_presenters/favorite.rb

@@ -1,7 +1,8 @@
 module PostSetPresenters
   class Favorite < Base
     attr_accessor :post_set, :tag_set_presenter
-    delegate :favorites, :to => :post_set
+    delegate :favorites, :posts, :to => :post_set
+    delegate :tag_list_html, to: :tag_set_presenter
 
     def initialize(post_set)
       @post_set = post_set
@@ -11,13 +12,5 @@ module PostSetPresenters
         ).map {|x| x[0]}
       )
     end
-
-    def tag_list_html(template)
-      tag_set_presenter.tag_list_html(template)
-    end
-
-    def posts
-      @posts ||= post_set.posts
-    end
   end
 end

app/presenters/post_set_presenters/pool.rb

@@ -12,10 +12,6 @@ module PostSetPresenters
       )
     end
 
-    def tag_list_html(template)
-      tag_set_presenter.tag_list_html(template)
-    end
-
     def post_previews_html(template)
       html = ""
 

app/presenters/post_set_presenters/post.rb

@@ -67,12 +67,8 @@ module PostSetPresenters
       SavedSearch.labels_for(CurrentUser.user.id).map {|x| "search:#{x}"}
     end
 
-    def tag_list_html(template, options = {})
+    def tag_list_html(**options)
-      if post_set.is_saved_search?
+      tag_set_presenter.tag_list_html(name_only: post_set.is_saved_search?, **options)
-        options[:name_only] = true
-      end
-      
-      tag_set_presenter.tag_list_html(template, options)
     end
   end
 end

app/presenters/tag_set_presenter.rb

@@ -5,16 +5,23 @@
 =end
 
 class TagSetPresenter < Presenter
-  def initialize(tags)
+  extend Memoist
-    @tags = tags
+  attr_reader :tag_names
+
+  # @param [Array<String>] a list of tags to present. Tags will be presented in
+  # the order given. The list should not contain duplicates. The list may
+  # contain tags that do not exist in the tags table, such as metatags.
+  def initialize(tag_names)
+    @tag_names = tag_names
   end
 
-  def tag_list_html(template, options = {})
+  def tag_list_html(current_query: "", show_extra_links: false, name_only: false)
     html = ""
-    if @tags.present?
+
+    if ordered_tags.present?
       html << '<ul itemscope itemtype="http://schema.org/ImageObject">'
-      @tags.each do |tag|
+      ordered_tags.each do |tag|
-        html << build_list_item(tag, template, options)
+        html << build_list_item(tag, current_query: current_query, show_extra_links: show_extra_links, name_only: name_only)
       end
       html << "</ul>"
     end
@@ -22,16 +29,17 @@ class TagSetPresenter < Presenter
     html.html_safe
   end
 
-  def split_tag_list_html(template, category_list: TagCategory.split_header_list, headers: true, **options)
+  def split_tag_list_html(headers: true, category_list: TagCategory.split_header_list, current_query: "", show_extra_links: false, name_only: false, humanize_tags: true)
     html = ""
 
     category_list.each do |category|
-      typetags = typed_tags(category)
+      typetags = tags_for_category(category)
+
       if typetags.any?
         html << TagCategory.header_mapping[category] if headers
         html << %{<ul class="#{category}-tag-list">}
         typetags.each do |tag|
-          html << build_list_item(tag, template, **options)
+          html << build_list_item(tag, current_query: current_query, show_extra_links: show_extra_links, name_only: name_only, humanize_tags: humanize_tags)
         end
         html << "</ul>"
       end
@@ -41,69 +49,105 @@ class TagSetPresenter < Presenter
   end
 
   # compact (horizontal) list, as seen in the /comments index.
-  def inline_tag_list_html(template, classes: "inline-tag-list", **options)
+  def inline_tag_list_html(humanize_tags: true)
-    html = split_tag_list_html(template, category_list: TagCategory.categorized_list, headers: false, name_only: true, humanize_tags: true, **options)
+    html = split_tag_list_html(category_list: TagCategory.categorized_list, headers: false, show_extra_links: false, name_only: true, humanize_tags: humanize_tags)
-    template.tag.span(html, class: classes)
+    %{<span class="inline-tag-list">#{html}</span>}.html_safe
+  end
+
+  # the list of tags inside the tag box in the post edit form.
+  def split_tag_list_text(category_list: TagCategory.categorized_list)
+    category_list.map do |category|
+      tags_for_category(category).map(&:name).join(" ")
+    end.join(" \n")
+  end
+
+  def humanized_essential_tag_string(category_list: TagCategory.humanized_list, default: "")
+    strings = category_list.map do |category|
+      mapping = TagCategory.humanized_mapping[category]
+      max_tags = mapping["slice"]
+      regexmap = mapping["regexmap"]
+      formatstr = mapping["formatstr"]
+      excluded_tags = mapping["exclusion"]
+
+      type_tags = tags_for_category(category).map(&:name) - excluded_tags
+      next if type_tags.empty?
+
+      if max_tags > 0 && type_tags.length > max_tags
+        type_tags = type_tags.take(max_tags) + ["others"]
+      end
+
+      if regexmap != //
+        type_tags = type_tags.map { |tag| tag.match(regexmap)[1] }
+      end
+
+      if category == "copyright" && tags_for_category("character").blank?
+        type_tags.to_sentence
+      else
+        formatstr % type_tags.to_sentence
+      end
+    end
+
+    strings = strings.compact.join(" ").tr("_", " ")
+    strings.blank? ? default : strings
   end
 
   private
 
-  def typed_tags(name)
+  def tags
-    @typed_tags ||= {}
+    Tag.where(name: tag_names).select(:name, :post_count, :category)
-    @typed_tags[name] ||= begin
+  end
-      @tags.select do |tag|
+
-        categories[tag] == TagCategory.mapping[name]
+  def tags_by_category
-      end
+    ordered_tags.group_by(&:category)
+  end
+
+  def tags_for_category(category_name)
+    category = TagCategory.mapping[category_name.downcase]
+    tags_by_category[category] || []
+  end
+
+  def ordered_tags
+    names_to_tags = tags.map { |tag| [tag.name, tag] }.to_h
+
+    tag_names.map do |name|
+      names_to_tags[name] || Tag.new(name: name).freeze
     end
   end
 
-  def categories
+  def build_list_item(tag, name_only: false, humanize_tags: true, show_extra_links: false, current_query: "")
-    @categories ||= Tag.categories_for(@tags)
+    name = tag.name
-  end
+    count = tag.post_count
+    category = tag.category
 
-  def counts
+    html = %{<li class="category-#{tag.category}">}
-    @counts ||= Tag.counts_for(@tags).inject({}) do |hash, x|
-      hash[x["name"]] = x["post_count"]
-      hash
-    end
-  end
-
-  def build_list_item(tag, template, name_only: false, humanize_tags: true, show_extra_links: CurrentUser.is_gold?)
-    html = ""
-    html << %{<li class="category-#{categories[tag]}">}
-    current_query = template.params[:tags] || ""
 
     unless name_only
-      if categories[tag] == Tag.categories.artist
+      if category == Tag.categories.artist
-        html << %{<a class="wiki-link" href="/artists/show_or_new?name=#{u(tag)}">?</a> }
+        html << %{<a class="wiki-link" href="/artists/show_or_new?name=#{u(name)}">?</a> }
       else
-        html << %{<a class="wiki-link" href="/wiki_pages/show_or_new?title=#{u(tag)}">?</a> }
+        html << %{<a class="wiki-link" href="/wiki_pages/show_or_new?title=#{u(name)}">?</a> }
       end
 
       if show_extra_links && current_query.present?
-        html << %{<a rel="nofollow" href="/posts?tags=#{u(current_query)}+#{u(tag)}" class="search-inc-tag">+</a> }
+        html << %{<a rel="nofollow" href="/posts?tags=#{u(current_query)}+#{u(name)}" class="search-inc-tag">+</a> }
-        html << %{<a rel="nofollow" href="/posts?tags=#{u(current_query)}+-#{u(tag)}" class="search-exl-tag">&ndash;</a> }
+        html << %{<a rel="nofollow" href="/posts?tags=#{u(current_query)}+-#{u(name)}" class="search-exl-tag">&ndash;</a> }
       end
     end
 
-    humanized_tag = humanize_tags ? tag.tr("_", " ") : tag
+    humanized_tag = humanize_tags ? name.tr("_", " ") : name
-    if categories[tag] == Tag.categories.artist
+    itemprop = 'itemprop="author"' if category == Tag.categories.artist
-      itemprop = 'itemprop="author"'
+    html << %{<a class="search-tag" #{itemprop} href="/posts?tags=#{u(name)}">#{h(humanized_tag)}</a> }
-    else
-      itemprop = nil
-    end
-    html << %{<a class="search-tag" #{itemprop} href="/posts?tags=#{u(tag)}">#{h(humanized_tag)}</a> }
 
-    unless name_only
+    unless name_only || tag.new_record?
-      if counts[tag].to_i >= 10_000
+      if count >= 10_000
-        post_count = "#{counts[tag].to_i / 1_000}k"
+        post_count = "#{count / 1_000}k"
-      elsif counts[tag].to_i >= 1_000
+      elsif count >= 1_000
-        post_count = "%.1fk" % (counts[tag].to_f / 1_000)
+        post_count = "%.1fk" % (count / 1_000.0)
       else
-        post_count = counts[tag].to_s
+        post_count = count
       end
 
-      is_underused_tag = counts[tag].to_i <= 1 && categories[tag] == Tag.categories.general
+      is_underused_tag = count <= 1 && category == Tag.categories.general
       klass = "post-count#{is_underused_tag ? " low-post-count" : ""}"
       title = "New general tag detected. Check the spelling or populate it now."
 
@@ -113,4 +157,6 @@ class TagSetPresenter < Presenter
     html << "</li>"
     html
   end
+
+  memoize :tags, :tags_by_category, :ordered_tags, :humanized_essential_tag_string
 end

app/presenters/upload_presenter.rb

@@ -1,5 +1,12 @@
 class UploadPresenter < Presenter
+  attr_reader :upload
+  delegate :inline_tag_list_html, to: :tag_set_presenter
+
   def initialize(upload)
     @upload = upload
   end
+
+  def tag_set_presenter
+    @tag_set_presenter ||= TagSetPresenter.new(upload.tag_string.split)
+  end
 end

app/views/comments/index.atom.builder

@@ -9,7 +9,7 @@ atom_feed(root_url: comments_url(host: Danbooru.config.hostname)) do |feed|
 
   @comments.each do |comment|
     feed.entry(comment, published: comment.created_at, updated: comment.updated_at) do |entry|
-      entry.title("@#{comment.creator_name} on post ##{comment.post_id} (#{comment.post.humanized_essential_tag_string})")
+      entry.title("@#{comment.creator_name} on post ##{comment.post_id} (#{comment.post.presenter.humanized_essential_tag_string})")
       entry.content(<<-EOS.strip_heredoc, type: "html")
         <img src="#{comment.post.preview_file_url}"/>
 

app/views/comments/partials/index/_header.html.erb

@@ -26,7 +26,7 @@
   </div>
   <div class="row list-of-tags">
     <strong>Tags</strong>
-    <%= post.presenter.inline_tag_list_html(self) %>
+    <%= post.presenter.inline_tag_list_html %>
   </div>
 </div>
 

app/views/favorites/index.html.erb

@@ -9,7 +9,7 @@
 
       <section id="tag-box">
         <h1>Tags</h1>
-        <%= @favorite_set.presenter.tag_list_html(self) %>
+        <%= @favorite_set.presenter.tag_list_html %>
       </section>
 
       <section id="related-box">

app/views/moderator/post/queues/_post.html.erb

@@ -31,7 +31,7 @@
       <% if post.has_active_pools? %>
         <li><strong>Pools</strong>: <%= render "pools/inline_list", pools: post.pools.undeleted %></li>
       <% end %>
-      <li><strong>Tags</strong>: <%= post.presenter.inline_tag_list_html(self) %></li>
+      <li><strong>Tags</strong>: <%= post.presenter.inline_tag_list_html %></li>
     </ul>
   </section>
 </div>

app/views/posts/index.html.erb

@@ -9,7 +9,7 @@
 
       <section id="tag-box">
         <h1>Tags</h1>
-        <%= @post_set.presenter.tag_list_html(self) %>
+        <%= @post_set.presenter.tag_list_html(current_query: params[:tags], show_extra_links: CurrentUser.user.is_gold?) %>
       </section>
 
       <%= render "posts/partials/index/options" %>

app/views/posts/partials/show/_edit.html.erb

@@ -78,7 +78,7 @@
   <div class="input">
     <div>
       <%= f.label :tag_string, "Tags" %>
-      <%= f.text_area :tag_string, :size => "50x5", :value => post.presenter.categorized_tag_string + " ", :data => { :autocomplete => "tag-edit" } %>
+      <%= f.text_area :tag_string, :size => "50x5", :value => post.presenter.split_tag_list_text + " ", :data => { :autocomplete => "tag-edit" } %>
       <span id="open-edit-dialog" class="ui-icon ui-icon-arrow-1-ne" title="detach" style="display: none;" data-shortcut="shift+e"/>
     </div>
 

app/views/posts/show.html+tooltip.erb

@@ -46,5 +46,5 @@
     <% end %>
   </div>
 
-  <%= @post.presenter.inline_tag_list_html(self, humanize_tags: false) %>
+  <%= @post.presenter.inline_tag_list_html(humanize_tags: false) %>
 </div>

app/views/posts/show.html.erb

@@ -6,7 +6,7 @@
       <%= render "posts/partials/index/blacklist" %>
 
       <section id="tag-list">
-        <%= @post.presenter.split_tag_list_html(self) %>
+        <%= @post.presenter.split_tag_list_html(current_query: params[:tags], show_extra_links: CurrentUser.user.is_gold?) %>
       </section>
 
       <section id="post-information">

app/views/uploads/index.html.erb

@@ -57,7 +57,7 @@
 
               <span class="info">
                 <strong>Tags</strong>
-                <%= TagSetPresenter.new(upload.tag_string.split).inline_tag_list_html(self) %>
+                <%= upload.presenter.inline_tag_list_html %>
               </span>
             </td>
             <td>