From d0f060d8eb0ec267b78a756692dfa47ff5a895c0 Mon Sep 17 00:00:00 2001
From: evazion <noizave@gmail.com>
Date: Sun, 8 Sep 2019 23:28:02 -0500
Subject: [PATCH] api: refactor api attribute declarations.

Replace the `method_attributes` and `hidden_attributes` methods with
`api_attributes`. `api_attributes` can be used as a class macro:

    # include only the given attributes.
    api_attributes :id, :created_at, :creator_name, ...

    # include all default attributes plus the `creator_name` method.
    api_attributes including: [:creator_name]

or as an instance method:

    def api_attributes
       [:id, :created_at, :creator_name, ...]
    end

By default, all attributes are included except for IP addresses and
tsvector columns.
---
 app/controllers/reports_controller.rb         |  2 +
 app/logical/application_responder.rb          |  2 +-
 app/logical/reports/upload_tags.rb            | 23 +-------
 app/models/application_record.rb              | 56 +++++++++++--------
 app/models/comment.rb                         | 10 +---
 app/models/dmail.rb                           | 13 +----
 app/models/forum_post.rb                      | 23 --------
 app/models/forum_topic.rb                     | 20 -------
 app/models/mod_action.rb                      |  6 +-
 app/models/note.rb                            | 13 +----
 app/models/pool.rb                            |  6 +-
 app/models/post.rb                            | 24 +++-----
 app/models/post_appeal.rb                     |  6 +-
 app/models/post_archive.rb                    |  2 +-
 app/models/post_flag.rb                       | 17 ++----
 app/models/upload.rb                          |  9 +--
 app/models/user.rb                            | 29 ++++------
 app/models/user_name_change_request.rb        | 10 ++--
 app/models/wiki_page.rb                       | 13 +----
 .../active_record_api_extensions.rb           |  5 --
 test/functional/posts_controller_test.rb      | 13 +++++
 test/functional/users_controller_test.rb      |  5 +-
 22 files changed, 98 insertions(+), 209 deletions(-)
 delete mode 100644 config/initializers/active_record_api_extensions.rb

diff --git a/app/controllers/reports_controller.rb b/app/controllers/reports_controller.rb
index bef3f9fe7..f856a26e9 100644
--- a/app/controllers/reports_controller.rb
+++ b/app/controllers/reports_controller.rb
@@ -1,6 +1,7 @@
 class ReportsController < ApplicationController
   before_action :member_only, :except => [:upload_tags]
   before_action :moderator_only, :only => [:post_versions, :post_versions_create, :down_voting_post_report, :down_voting_post_report_create]
+  respond_to :html, :xml, :json, only: [:upload_tags]
 
   def uploads
     @report = Reports::Uploads.new(params[:min_date], params[:max_date], params[:queries])
@@ -16,6 +17,7 @@ class ReportsController < ApplicationController
   def upload_tags
     @user = User.find(params[:user_id])
     @upload_reports = Reports::UploadTags.includes(versions: { post: :versions }).for_user(params[:user_id]).order("id desc").paginate(params[:page], :limit => params[:limit])
+    respond_with(@upload_reports)
   end
 
   def down_voting_post
diff --git a/app/logical/application_responder.rb b/app/logical/application_responder.rb
index eb9baac34..1ac4a1d75 100644
--- a/app/logical/application_responder.rb
+++ b/app/logical/application_responder.rb
@@ -17,7 +17,7 @@ class ApplicationResponder < ActionController::Responder
       options[:root] ||= resource.table_name.dasherize if resource.respond_to?(:table_name)
     end
 
-    options[:only] ||= params["only"].scan(/\w+/) if params["only"]
+    options[:only] ||= params["only"].scan(/\w+/).map(&:to_sym) if params["only"]
 
     super
   end
diff --git a/app/logical/reports/upload_tags.rb b/app/logical/reports/upload_tags.rb
index 65bc813c8..c7b4ab3aa 100644
--- a/app/logical/reports/upload_tags.rb
+++ b/app/logical/reports/upload_tags.rb
@@ -1,32 +1,13 @@
 module Reports
   class UploadTags < ::Post
-
     def readonly?
       true
     end
 
-    module ApiMethods
-
-      def as_json(options = {})
-        options ||= {}
-        options[:only] ||= [:id]
-        super(options)
-      end
-
-      def to_xml(options = {}, &block)
-        options ||= {}
-        options[:only] ||= [:id, :uploader_id]
-        super(options, &block)
-      end
-
-      def method_attributes
-        [:uploader_tags, :added_tags, :removed_tags]
-      end
-
+    def api_attributes
+      [:id, :uploader_id, :uploader_tags, :added_tags, :removed_tags]
     end
 
-    include ApiMethods
-
     def uploader_tags_array
       @uploader_tags ||= begin
         uploader_versions = versions.where(updater_id: uploader_id)
diff --git a/app/models/application_record.rb b/app/models/application_record.rb
index c5bff32e8..0307da882 100644
--- a/app/models/application_record.rb
+++ b/app/models/application_record.rb
@@ -215,35 +215,47 @@ class ApplicationRecord < ActiveRecord::Base
   module ApiMethods
     extend ActiveSupport::Concern
 
-    def serializable_hash(options = {})
-      options ||= {}
+    class_methods do
+      def api_attributes(*attributes, including: [])
+        return @api_attributes if @api_attributes
 
-      options[:include] ||= []
+        if attributes.present?
+          @api_attributes = attributes
+        else
+          @api_attributes = attribute_types.reject { |name, attr| attr.type.in?([:inet, :tsvector]) }.keys.map(&:to_sym)
+        end
 
-      options[:except] ||= []
-      options[:except] += hidden_attributes
-
-      options[:methods] ||= []
-      options[:methods] += method_attributes
-
-      if options[:only]
-        options[:methods] = options[:methods] & options[:only].map(&:to_sym)
-        options[:include] = options[:include] & options[:only].map(&:to_sym)
+        @api_attributes += including
+        @api_attributes
       end
+    end
+
+    def api_attributes
+      self.class.api_attributes
+    end
+
+    def serializable_hash(options = {})
+      options[:only] ||= []
+      options[:include] ||= []
+      options[:methods] ||= []
+
+      attributes, methods = api_attributes.partition { |attr| has_attribute?(attr) }
+      methods += options[:methods]
+      includes = options[:include]
+
+      if options[:only].present?
+        attributes &= options[:only]
+        methods &= options[:only]
+        includes &= options[:only]
+      end
+
+      options[:only] = attributes
+      options[:methods] = methods
+      options[:include] = includes
 
       hash = super(options)
       hash.transform_keys { |key| key.delete("?") }
     end
-
-    protected
-
-    def hidden_attributes
-      [:uploader_ip_addr, :updater_ip_addr, :creator_ip_addr]
-    end
-
-    def method_attributes
-      []
-    end
   end
 
   concerning :ActiveRecordExtensions do
diff --git a/app/models/comment.rb b/app/models/comment.rb
index f24e2e5bc..eb3779747 100644
--- a/app/models/comment.rb
+++ b/app/models/comment.rb
@@ -22,6 +22,8 @@ class Comment < ApplicationRecord
     :body => ->(user_name) {"@#{creator.name} mentioned you in a \"comment\":/posts/#{post_id}#comment-#{id} on post ##{post_id}:\n\n[quote]\n#{DText.excerpt(body, "@"+user_name)}\n[/quote]\n"},
   )
 
+  api_attributes including: [:creator_name, :updater_name]
+
   module SearchMethods
     def deleted
       where("comments.is_deleted = true")
@@ -145,14 +147,6 @@ class Comment < ApplicationRecord
     select { |comment| comment.visibility(user) == :visible }
   end
 
-  def hidden_attributes
-    super + [:body_index]
-  end
-
-  def method_attributes
-    super + [:creator_name, :updater_name]
-  end
-
   def creator_name
     creator.name
   end
diff --git a/app/models/dmail.rb b/app/models/dmail.rb
index 66d182765..212426c4b 100644
--- a/app/models/dmail.rb
+++ b/app/models/dmail.rb
@@ -18,6 +18,8 @@ class Dmail < ApplicationRecord
   after_create :update_recipient
   after_commit :send_email, on: :create
 
+  api_attributes including: [:key]
+
   concerning :SpamMethods do
     class_methods do
       def is_spammer?(user)
@@ -103,16 +105,6 @@ class Dmail < ApplicationRecord
     end
   end
 
-  module ApiMethods
-    def hidden_attributes
-      super + [:message_index]
-    end
-    
-    def method_attributes
-      super + [:key]
-    end
-  end
-  
   module SearchMethods
     def sent_by(user)
       where("dmails.from_id = ? AND dmails.owner_id != ?", user.id, user.id)
@@ -156,7 +148,6 @@ class Dmail < ApplicationRecord
 
   include AddressMethods
   include FactoryMethods
-  include ApiMethods
   extend SearchMethods
 
   def validate_sender_is_not_banned
diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb
index 6aa3bbe4b..e0aa4ce87 100644
--- a/app/models/forum_post.rb
+++ b/app/models/forum_post.rb
@@ -62,30 +62,7 @@ class ForumPost < ApplicationRecord
     end
   end
 
-  module ApiMethods
-    def as_json(options = {})
-      if CurrentUser.user.level < topic.min_level
-        options[:only] = [:id]
-      end
-
-      super(options)
-    end
-
-    def to_xml(options = {})
-      if CurrentUser.user.level < topic.min_level
-        options[:only] = [:id]
-      end
-
-      super(options)
-    end
-    
-    def hidden_attributes
-      super + [:text_index]
-    end
-  end
-
   extend SearchMethods
-  include ApiMethods
 
   def self.new_reply(params)
     if params[:topic_id]
diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb
index f24d22dc4..c57219841 100644
--- a/app/models/forum_topic.rb
+++ b/app/models/forum_topic.rb
@@ -155,26 +155,6 @@ class ForumTopic < ApplicationRecord
     (response_count / Danbooru.config.posts_per_page.to_f).ceil
   end
 
-  def as_json(options = {})
-    if CurrentUser.user.level < min_level
-      options[:only] = [:id]
-    end
-
-    super(options)
-  end
-
-  def to_xml(options = {})
-    if CurrentUser.user.level < min_level
-      options[:only] = [:id]
-    end
-
-    super(options)
-  end
-
-  def hidden_attributes
-    super + [:text_index, :min_level]
-  end
-
   def merge(topic)
     ForumPost.where(:id => self.posts.map(&:id)).update_all(:topic_id => topic.id)
     topic.update(response_count: topic.response_count + self.posts.length, updater_id: CurrentUser.id)
diff --git a/app/models/mod_action.rb b/app/models/mod_action.rb
index b312f7f0a..698bd9b14 100644
--- a/app/models/mod_action.rb
+++ b/app/models/mod_action.rb
@@ -2,6 +2,8 @@ class ModAction < ApplicationRecord
   belongs_to :creator, :class_name => "User"
   before_validation :initialize_creator, :on => :create
 
+  api_attributes including: [:category_id]
+
   #####DIVISIONS#####
   #Groups:     0-999
   #Individual: 1000-1999
@@ -73,10 +75,6 @@ class ModAction < ApplicationRecord
     self.class.categories[category]
   end
 
-  def method_attributes
-    super + [:category_id]
-  end
-
   def serializable_hash(*args)
     super(*args).merge("description" => filtered_description)
   end
diff --git a/app/models/note.rb b/app/models/note.rb
index 25faf98cd..4ccae7aed 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -11,6 +11,8 @@ class Note < ApplicationRecord
   after_save :create_version
   validate :post_must_not_be_note_locked
 
+  api_attributes including: [:creator_name]
+
   module SearchMethods
     def active
       where("is_active = TRUE")
@@ -26,22 +28,11 @@ class Note < ApplicationRecord
     end
   end
 
-  module ApiMethods
-    def hidden_attributes
-      super + [:body_index]
-    end
-
-    def method_attributes
-      super + [:creator_name]
-    end
-  end
-
   def creator_name
     creator.name
   end
 
   extend SearchMethods
-  include ApiMethods
 
   def post_must_not_be_note_locked
     if is_locked?
diff --git a/app/models/pool.rb b/app/models/pool.rb
index a98688408..6699fad32 100644
--- a/app/models/pool.rb
+++ b/app/models/pool.rb
@@ -17,6 +17,8 @@ class Pool < ApplicationRecord
   after_save :create_version
   after_create :synchronize!
 
+  api_attributes including: [:creator_name, :post_count]
+
   module SearchMethods
     def deleted
       where("pools.is_deleted = true")
@@ -288,10 +290,6 @@ class Pool < ApplicationRecord
     (post_count / CurrentUser.user.per_page.to_f).ceil
   end
 
-  def method_attributes
-    super + [:creator_name, :post_count]
-  end
-
   def creator_name
     creator.name
   end
diff --git a/app/models/post.rb b/app/models/post.rb
index 845e9afdb..d07b15d60 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -1457,23 +1457,13 @@ class Post < ApplicationRecord
   end
 
   module ApiMethods
-    def hidden_attributes
-      list = super + [:tag_index]
-      unless CurrentUser.is_moderator?
-        list += [:fav_string]
-      end
-      if !visible?
-        list += [:md5, :file_ext]
-      end
-      super + list
-    end
-
-    def method_attributes
-      list = super + [:uploader_name, :has_large, :has_visible_children, :children_ids, :is_favorited?] + TagCategory.categories.map {|x| "tag_string_#{x}".to_sym}
-      if visible?
-        list += [:file_url, :large_file_url, :preview_file_url]
-      end
-      list
+    def api_attributes
+      attributes = super
+      attributes += [:uploader_name, :has_large, :has_visible_children, :children_ids, :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 associated_attributes
diff --git a/app/models/post_appeal.rb b/app/models/post_appeal.rb
index 36298f44c..371222e94 100644
--- a/app/models/post_appeal.rb
+++ b/app/models/post_appeal.rb
@@ -9,6 +9,8 @@ class PostAppeal < ApplicationRecord
   before_validation :initialize_creator, :on => :create
   validates_uniqueness_of :creator_id, :scope => :post_id, :message => "have already appealed this post"
 
+  api_attributes including: [:is_resolved]
+
   module SearchMethods
     def resolved
       joins(:post).where("posts.is_deleted = false and posts.is_flagged = false")
@@ -64,8 +66,4 @@ class PostAppeal < ApplicationRecord
   def appeal_count_for_creator
     creator.post_appeals.recent.count
   end
-
-  def method_attributes
-    super + [:is_resolved]
-  end
 end
diff --git a/app/models/post_archive.rb b/app/models/post_archive.rb
index 991187b9c..9410da237 100644
--- a/app/models/post_archive.rb
+++ b/app/models/post_archive.rb
@@ -260,7 +260,7 @@ class PostArchive < ApplicationRecord
     updater.name
   end
 
-  def method_attributes
+  def api_attributes
     super + [:obsolete_added_tags, :obsolete_removed_tags, :unchanged_tags, :updater_name]
   end
 
diff --git a/app/models/post_flag.rb b/app/models/post_flag.rb
index 37b41a6e3..3257fc036 100644
--- a/app/models/post_flag.rb
+++ b/app/models/post_flag.rb
@@ -94,22 +94,13 @@ class PostFlag < ApplicationRecord
     end
   end
 
-  module ApiMethods
-    def hidden_attributes
-      list = super
-      unless CurrentUser.can_view_flagger_on_post?(self)
-        list += [:creator_id]
-      end
-      super + list
-    end
-
-    def method_attributes
-      super + [:category]
-    end
+  def api_attributes
+    attributes = super + [:category]
+    attributes -= [:creator_id] unless CurrentUser.can_view_flagger_on_post?(self)
+    attributes
   end
 
   extend SearchMethods
-  include ApiMethods
 
   def category
     case reason
diff --git a/app/models/upload.rb b/app/models/upload.rb
index 511bf6af7..e931aac27 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -72,6 +72,8 @@ class Upload < ApplicationRecord
 
   scope :preprocessed, -> { where(status: "preprocessed") }
 
+  api_attributes including: [:uploader_name]
+
   def initialize_attributes
     self.uploader_id = CurrentUser.id
     self.uploader_ip_addr = CurrentUser.ip_addr
@@ -227,18 +229,11 @@ class Upload < ApplicationRecord
     end
   end
 
-  module ApiMethods
-    def method_attributes
-      super + [:uploader_name]
-    end
-  end
-
   include FileMethods
   include StatusMethods
   include UploaderMethods
   include VideoMethods
   extend SearchMethods
-  include ApiMethods
   include SourceMethods
 
   def uploader_is_not_limited
diff --git a/app/models/user.rb b/app/models/user.rb
index 959cdef37..c98f32a26 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -567,33 +567,28 @@ class User < ApplicationRecord
   end
 
   module ApiMethods
-    # blacklist all attributes by default. whitelist only safe attributes.
-    def hidden_attributes
-      super + attributes.keys.map(&:to_sym)
-    end
-
-    def method_attributes
-      list = super + [
+    def api_attributes
+      attributes = [
         :id, :created_at, :name, :inviter_id, :level, :base_upload_limit,
-        :post_upload_count, :post_update_count, :note_update_count,
-        :is_banned, :can_approve_posts, :can_upload_free, :is_super_voter,
-        :level_string,
+        :post_upload_count, :post_update_count, :note_update_count, :is_banned,
+        :can_approve_posts, :can_upload_free, :is_super_voter, :level_string,
       ]
 
       if id == CurrentUser.user.id
-        list += BOOLEAN_ATTRIBUTES + [
+        attributes += BOOLEAN_ATTRIBUTES
+        attributes += [
           :updated_at, :email, :last_logged_in_at, :last_forum_read_at,
           :recent_tags, :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, :can_comment_vote?, :can_remove_from_pools?,
-          :is_comment_limited?, :can_comment?, :can_upload?, :max_saved_searches,
+          :custom_style, :favorite_count, :api_regen_multiplier,
+          :api_burst_limit, :remaining_api_limit, :statement_timeout,
+          :favorite_group_limit, :favorite_limit, :tag_query_limit,
+          :can_comment_vote?, :can_remove_from_pools?, :is_comment_limited?,
+          :can_comment?, :can_upload?, :max_saved_searches,
         ]
       end
 
-      list
+      attributes
     end
 
     # extra attributes returned for /users/:id.json but not for /users.json.
diff --git a/app/models/user_name_change_request.rb b/app/models/user_name_change_request.rb
index ee6bb56a5..385090a8f 100644
--- a/app/models/user_name_change_request.rb
+++ b/app/models/user_name_change_request.rb
@@ -67,11 +67,9 @@ class UserNameChangeRequest < ApplicationRecord
     end
   end
 
-  def hidden_attributes
-    if CurrentUser.is_admin? || user == CurrentUser.user
-      []
-    else
-      super + [:change_reason, :rejection_reason]
-    end
+  def api_attributes
+    attributes = super
+    attributes -= [:change_reason, :rejection_reason] unless CurrentUser.is_admin? || user == CurrentUser.user
+    attributes
   end
 end
diff --git a/app/models/wiki_page.rb b/app/models/wiki_page.rb
index 7ab62abf5..afe64d5ec 100644
--- a/app/models/wiki_page.rb
+++ b/app/models/wiki_page.rb
@@ -18,6 +18,8 @@ class WikiPage < ApplicationRecord
   has_one :artist, -> {where(:is_active => true)}, :foreign_key => "name", :primary_key => "title"
   has_many :versions, -> {order("wiki_page_versions.id ASC")}, :class_name => "WikiPageVersion", :dependent => :destroy
 
+  api_attributes including: [:creator_name, :category_name]
+
   module SearchMethods
     def titled(title)
       where("title = ?", title.mb_chars.downcase.tr(" ", "_"))
@@ -88,22 +90,11 @@ class WikiPage < ApplicationRecord
     end
   end
 
-  module ApiMethods
-    def hidden_attributes
-      super + [:body_index]
-    end
-
-    def method_attributes
-      super + [:creator_name, :category_name]
-    end
-  end
-
   def creator_name
     creator.name
   end
 
   extend SearchMethods
-  include ApiMethods
 
   def validate_not_locked
     if is_locked? && !CurrentUser.is_builder?
diff --git a/config/initializers/active_record_api_extensions.rb b/config/initializers/active_record_api_extensions.rb
deleted file mode 100644
index 5c88773d3..000000000
--- a/config/initializers/active_record_api_extensions.rb
+++ /dev/null
@@ -1,5 +0,0 @@
-class Delayed::Job
-  def hidden_attributes
-    [:handler]
-  end
-end
diff --git a/test/functional/posts_controller_test.rb b/test/functional/posts_controller_test.rb
index e7785fdf5..a54f69f13 100644
--- a/test/functional/posts_controller_test.rb
+++ b/test/functional/posts_controller_test.rb
@@ -217,6 +217,19 @@ class PostsControllerTest < ActionDispatch::IntegrationTest
           assert_response :success
         end
       end
+
+      context "in api responses" do
+        should "not include restricted attributes" do
+          as(@user) { @post.update(tag_string: "loli") }
+          get_auth post_path(@post), @user, as: :json
+
+          assert_response :success
+          assert_nil(response.parsed_body["md5"])
+          assert_nil(response.parsed_body["file_url"])
+          assert_nil(response.parsed_body["fav_string"])
+          assert_equal(@post.uploader_name, response.parsed_body["uploader_name"])
+        end
+      end
     end
 
     context "update action" do
diff --git a/test/functional/users_controller_test.rb b/test/functional/users_controller_test.rb
index d21682780..07155057e 100644
--- a/test/functional/users_controller_test.rb
+++ b/test/functional/users_controller_test.rb
@@ -84,11 +84,10 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
       end
 
       should "render the current users's profile in json" do
-        get_auth profile_path(format: :json), @user
+        get_auth profile_path, @user, as: :json
         assert_response :success
 
-        json = as(@user) { @user.as_json(methods: @user.full_attributes + @user.method_attributes) }
-        assert_equal(json, response.parsed_body)
+        assert_equal(@user.upload_limit, response.parsed_body["upload_limit"])
       end
 
       should "redirect anonymous users to the sign in page" do