From eacb4d4df337c63c9edad2612c2e19594cc6c309 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Mon, 8 Jun 2020 18:38:02 -0500
Subject: [PATCH] models: factor out `api_attributes` to policies.

Refactor models so that we define attribute API permissions in policy
files instead of directly in models.

This is cleaner because a) permissions are better handled by policies
and b) which attributes are visible to the API is an API-level concern
that models shouldn't have to care about.

This fixes an issue with not being able to precompile CSS/JS assets
unless the database was up and running. This was a problem when building
Docker images because we don't have a database at build time. We needed
the database because `api_attributes` was a class-level macro in some
places, which meant it ran at boot time, but this triggered a database
call because api_attributes used database introspection to get the list
of allowed API attributes.
---
 app/models/application_record.rb        | 18 +++---------------
 app/models/dmail.rb                     |  1 -
 app/models/mod_action.rb                |  2 --
 app/models/pool.rb                      |  1 -
 app/models/post.rb                      | 21 ++++++---------------
 app/models/post_disapproval.rb          |  6 ------
 app/models/post_flag.rb                 |  9 ---------
 app/models/post_version.rb              |  4 ----
 app/models/user.rb                      | 24 ------------------------
 app/models/wiki_page.rb                 |  1 -
 app/policies/application_policy.rb      |  4 ++++
 app/policies/dmail_policy.rb            |  4 ++++
 app/policies/mod_action_policy.rb       |  5 +++++
 app/policies/pool_policy.rb             |  4 ++++
 app/policies/post_disapproval_policy.rb |  6 ++++++
 app/policies/post_flag_policy.rb        |  6 ++++++
 app/policies/post_policy.rb             | 12 +++++++++++-
 app/policies/post_version_policy.rb     |  4 ++++
 app/policies/user_policy.rb             | 24 ++++++++++++++++++++++++
 app/policies/wiki_page_policy.rb        |  4 ++++
 20 files changed, 81 insertions(+), 79 deletions(-)
 create mode 100644 app/policies/mod_action_policy.rb

diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index c844350a9..24ec27a76 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -31,19 +31,6 @@ class ApplicationRecord < ActiveRecord::Base
 
   concerning :ApiMethods do
     class_methods do
-      def api_attributes(*attributes, including: [])
-        return @api_attributes if @api_attributes
-
-        if attributes.present?
-          @api_attributes = attributes
-        else
-          @api_attributes = attribute_types.reject { |name, attr| attr.type.in?([:inet, :tsvector]) }.keys.map(&:to_sym)
-        end
-
-        @api_attributes += including
-        @api_attributes
-      end
-
       def available_includes
         []
       end
@@ -65,8 +52,9 @@ class ApplicationRecord < ActiveRecord::Base
       self.class.available_includes
     end
 
-    def api_attributes
-      self.class.api_attributes
+    # XXX deprecated, shouldn't expose this as an instance method.
+    def api_attributes(user: CurrentUser.user)
+      Pundit.policy!([user, nil], self).api_attributes
     end
 
     def html_data_attributes
diff --git a/app/models/dmail.rb b/app/models/dmail.rb
index a155582ff..25c053213 100644
--- a/app/models/dmail.rb
+++ b/app/models/dmail.rb
@@ -12,7 +12,6 @@ class Dmail < ApplicationRecord
   after_save :update_unread_dmail_count
   after_commit :send_email, on: :create
 
-  api_attributes including: [:key]
   deletable
 
   scope :read, -> { where(is_read: true) }
diff --git a/app/models/mod_action.rb b/app/models/mod_action.rb
index 2a7c42f92..73b3f58f9 100644
--- a/app/models/mod_action.rb
+++ b/app/models/mod_action.rb
@@ -1,8 +1,6 @@
 class ModAction < ApplicationRecord
   belongs_to :creator, :class_name => "User"
 
-  api_attributes including: [:category_id]
-
   # ####DIVISIONS#####
   # Groups:     0-999
   # Individual: 1000-1999
diff --git a/app/models/pool.rb b/app/models/pool.rb
index 0d1a579ab..e789f10a7 100644
--- a/app/models/pool.rb
+++ b/app/models/pool.rb
@@ -13,7 +13,6 @@ class Pool < ApplicationRecord
   after_save :create_version
   after_create :synchronize!
 
-  api_attributes including: [:post_count]
   deletable
 
   scope :series, -> { where(category: "series") }
diff --git a/app/models/post.rb b/app/models/post.rb
index e37453447..e1d40ef91 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -1125,15 +1125,6 @@ class Post < ApplicationRecord
   end
 
   module ApiMethods
-    def api_attributes
-      attributes = super
-      attributes += [:has_large, :has_visible_children, :is_favorited?] + TagCategory.categories.map {|x| "tag_string_#{x}".to_sym}
-      attributes += [:file_url, :large_file_url, :preview_file_url] if visible?
-      attributes -= [:md5, :file_ext] if !visible?
-      attributes -= [:fav_string] if !CurrentUser.is_moderator?
-      attributes
-    end
-
     def legacy_attributes
       hash = {
         "has_comments" => last_commented_at.present?,
@@ -1471,16 +1462,16 @@ class Post < ApplicationRecord
     CurrentUser.safe_mode? && (rating != "s" || Danbooru.config.safe_mode_restricted_tags.any? { |tag| tag.in?(tag_array) })
   end
 
-  def levelblocked?
-    !CurrentUser.is_gold? && Danbooru.config.restricted_tags.any? { |tag| tag.in?(tag_array) }
+  def levelblocked?(user = CurrentUser.user)
+    !user.is_gold? && Danbooru.config.restricted_tags.any? { |tag| tag.in?(tag_array) }
   end
 
-  def banblocked?
-    is_banned? && !CurrentUser.is_gold?
+  def banblocked?(user = CurrentUser.user)
+    is_banned? && !user.is_gold?
   end
 
-  def visible?
-    !safeblocked? && !levelblocked? && !banblocked?
+  def visible?(user = CurrentUser.user)
+    !safeblocked? && !levelblocked?(user) && !banblocked?(user)
   end
 
   def reload(options = nil)
diff --git a/app/models/post_disapproval.rb b/app/models/post_disapproval.rb
index ae18c98b4..6e594c4cb 100644
--- a/app/models/post_disapproval.rb
+++ b/app/models/post_disapproval.rb
@@ -73,10 +73,4 @@ class PostDisapproval < ApplicationRecord
     message = nil if message.blank?
     super(message)
   end
-
-  def api_attributes
-    attributes = super
-    attributes -= [:creator_id] unless Pundit.policy!([CurrentUser.user, nil], self).can_view_creator?
-    attributes
-  end
 end
diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb
index f87f512c4..ff928bbee 100644
--- a/app/models/post_flag.rb
+++ b/app/models/post_flag.rb
@@ -75,16 +75,7 @@ class PostFlag < ApplicationRecord
     end
   end
 
-  module ApiMethods
-    def api_attributes
-      attributes = super + [:category]
-      attributes -= [:creator_id] unless Pundit.policy!([CurrentUser.user, nil], self).can_view_flagger?
-      attributes
-    end
-  end
-
   extend SearchMethods
-  include ApiMethods
 
   def category
     case reason
diff --git a/app/models/post_version.rb b/app/models/post_version.rb
index 701742d67..a6be23df2 100644
--- a/app/models/post_version.rb
+++ b/app/models/post_version.rb
@@ -267,10 +267,6 @@ class PostVersion < ApplicationRecord
     post.save!
   end
 
-  def api_attributes
-    super + [:obsolete_added_tags, :obsolete_removed_tags, :unchanged_tags]
-  end
-
   def self.available_includes
     [:updater, :post]
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index 6b3e3929c..064b8a6c3 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -420,30 +420,6 @@ class User < ApplicationRecord
   end
 
   module ApiMethods
-    def api_attributes
-      attributes = %i[
-        id created_at name inviter_id level
-        post_upload_count post_update_count note_update_count is_banned
-        can_approve_posts can_upload_free level_string
-      ]
-
-      if id == CurrentUser.user.id
-        attributes += BOOLEAN_ATTRIBUTES
-        attributes += %i[
-          updated_at last_logged_in_at last_forum_read_at
-          comment_threshold default_image_size
-          favorite_tags blacklisted_tags time_zone per_page
-          custom_style favorite_count api_regen_multiplier
-          api_burst_limit remaining_api_limit statement_timeout
-          favorite_group_limit favorite_limit tag_query_limit
-          is_comment_limited?
-          max_saved_searches theme
-        ]
-      end
-
-      attributes
-    end
-
     # extra attributes returned for /users/:id.json but not for /users.json.
     def full_attributes
       %i[
diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb
index b0ea6b766..efef59f3a 100644
--- a/app/models/wiki_page.rb
+++ b/app/models/wiki_page.rb
@@ -18,7 +18,6 @@ class WikiPage < ApplicationRecord
   has_many :versions, -> {order("wiki_page_versions.id ASC")}, :class_name => "WikiPageVersion", :dependent => :destroy
   has_many :dtext_links, as: :model, dependent: :destroy
 
-  api_attributes including: [:category_name]
   deletable
 
   module SearchMethods
diff --git a/app/policies/application_policy.rb b/app/policies/application_policy.rb
index b834eec02..c4c123547 100644
--- a/app/policies/application_policy.rb
+++ b/app/policies/application_policy.rb
@@ -69,4 +69,8 @@ class ApplicationPolicy
   def permitted_attributes_for_edit
     permitted_attributes_for_update
   end
+
+  def api_attributes
+    record.class.attribute_types.reject { |name, attr| attr.type.in?([:inet, :tsvector]) }.keys.map(&:to_sym)
+  end
 end
diff --git a/app/policies/dmail_policy.rb b/app/policies/dmail_policy.rb
index 5dc03a0b5..5d97e9916 100644
--- a/app/policies/dmail_policy.rb
+++ b/app/policies/dmail_policy.rb
@@ -30,4 +30,8 @@ class DmailPolicy < ApplicationPolicy
   def permitted_attributes_for_update
     [:is_read, :is_deleted]
   end
+
+  def api_attributes
+    super + [:key]
+  end
 end
diff --git a/app/policies/mod_action_policy.rb b/app/policies/mod_action_policy.rb
new file mode 100644
index 000000000..8972a4bae
--- /dev/null
+++ b/app/policies/mod_action_policy.rb
@@ -0,0 +1,5 @@
+class ModActionPolicy < ApplicationPolicy
+  def api_attributes
+    super + [:category_id]
+  end
+end
diff --git a/app/policies/pool_policy.rb b/app/policies/pool_policy.rb
index cb76b67ed..4617c87d1 100644
--- a/app/policies/pool_policy.rb
+++ b/app/policies/pool_policy.rb
@@ -22,4 +22,8 @@ class PoolPolicy < ApplicationPolicy
   def permitted_attributes
     [:name, :description, :category, :post_ids, :post_ids_string, post_ids: []]
   end
+
+  def api_attributes
+    super + [:post_count]
+  end
 end
diff --git a/app/policies/post_disapproval_policy.rb b/app/policies/post_disapproval_policy.rb
index fad211267..e5f9b273f 100644
--- a/app/policies/post_disapproval_policy.rb
+++ b/app/policies/post_disapproval_policy.rb
@@ -10,4 +10,10 @@ class PostDisapprovalPolicy < ApplicationPolicy
   def permitted_attributes
     [:post_id, :reason, :message]
   end
+
+  def api_attributes
+    attributes = super
+    attributes -= [:creator_id] unless can_view_creator?
+    attributes
+  end
 end
diff --git a/app/policies/post_flag_policy.rb b/app/policies/post_flag_policy.rb
index 09207cbe5..d23f92f92 100644
--- a/app/policies/post_flag_policy.rb
+++ b/app/policies/post_flag_policy.rb
@@ -10,4 +10,10 @@ class PostFlagPolicy < ApplicationPolicy
   def permitted_attributes
     [:post_id, :reason]
   end
+
+  def api_attributes
+    attributes = super + [:category]
+    attributes -= [:creator_id] unless can_view_flagger?
+    attributes
+  end
 end
diff --git a/app/policies/post_policy.rb b/app/policies/post_policy.rb
index 42647bd8d..fa94e340a 100644
--- a/app/policies/post_policy.rb
+++ b/app/policies/post_policy.rb
@@ -44,7 +44,7 @@ class PostPolicy < ApplicationPolicy
   end
 
   def visible?
-    record.visible?
+    record.visible?(user)
   end
 
   def can_view_uploader?
@@ -85,4 +85,14 @@ class PostPolicy < ApplicationPolicy
       (:is_status_locked if can_lock_status?),
     ].compact
   end
+
+  def api_attributes
+    attributes = super
+    attributes += [:has_large, :has_visible_children, :is_favorited?]
+    attributes += TagCategory.categories.map {|x| "tag_string_#{x}".to_sym}
+    attributes += [:file_url, :large_file_url, :preview_file_url] if visible?
+    attributes -= [:md5, :file_ext] if !visible?
+    attributes -= [:fav_string] if !user.is_moderator?
+    attributes
+  end
 end
diff --git a/app/policies/post_version_policy.rb b/app/policies/post_version_policy.rb
index 781a7cc8f..199cf100b 100644
--- a/app/policies/post_version_policy.rb
+++ b/app/policies/post_version_policy.rb
@@ -6,4 +6,8 @@ class PostVersionPolicy < ApplicationPolicy
   def can_mass_undo?
     user.is_builder?
   end
+
+  def api_attributes
+    super + [:obsolete_added_tags, :obsolete_removed_tags, :unchanged_tags]
+  end
 end
diff --git a/app/policies/user_policy.rb b/app/policies/user_policy.rb
index e9c0f7915..2814fde68 100644
--- a/app/policies/user_policy.rb
+++ b/app/policies/user_policy.rb
@@ -51,6 +51,30 @@ class UserPolicy < ApplicationPolicy
     ].compact
   end
 
+  def api_attributes
+    attributes = %i[
+      id created_at name inviter_id level
+      post_upload_count post_update_count note_update_count is_banned
+      can_approve_posts can_upload_free level_string
+    ]
+
+    if record.id == user.id
+      attributes += User::BOOLEAN_ATTRIBUTES
+      attributes += %i[
+        updated_at last_logged_in_at last_forum_read_at
+        comment_threshold default_image_size
+        favorite_tags blacklisted_tags time_zone per_page
+        custom_style favorite_count api_regen_multiplier
+        api_burst_limit remaining_api_limit statement_timeout
+        favorite_group_limit favorite_limit tag_query_limit
+        is_comment_limited?
+        max_saved_searches theme
+      ]
+    end
+
+    attributes
+  end
+
   alias_method :profile?, :show?
   alias_method :settings?, :edit?
 end
diff --git a/app/policies/wiki_page_policy.rb b/app/policies/wiki_page_policy.rb
index 81b897fcc..c6e34850d 100644
--- a/app/policies/wiki_page_policy.rb
+++ b/app/policies/wiki_page_policy.rb
@@ -14,4 +14,8 @@ class WikiPagePolicy < ApplicationPolicy
   def permitted_attributes
     [:title, :body, :other_names, :other_names_string, :is_deleted, (:is_locked if can_edit_locked?)].compact
   end
+
+  def api_attributes
+    super + [:category_name]
+  end
 end