ronantakizawa/codereview-bench huggingface dataset:
Repository: rails/rails
File: railties/lib/rails/application/configuration.rb (Ruby)

Code before:

          if respond_to?(:active_record)
            active_record.raise_on_missing_required_finder_order_columns = true
          end

          if respond_to?(:active_support)
            active_support.escape_js_separators_in_json = false
          end

          if respond_to?(:action_view)
            action_view.render_tracker = :ruby
          end

          if respond_to?(:action_view)
            action_view.remove_hidden_field_autocomplete = true
          end
        when "8.2"
          load_defaults "8.1"

          if respond_to?(:active_record)
            active_record.postgresql_adapter_decode_bytea = true
            active_record.postgresql_adapter_decode_money = true
          end
        else
          raise "Unknown version #{target_version.to_s.inspect}"
        end

        @loaded_config_version = target_version
      end

      def reloading_enabled?
        enable_reloading
      end

      def enable_reloading
        !cache_classes
      end

      def enable_reloading=(value)
        self.cache_classes = !value
      end

      def encoding=(value)
        @encoding = value
        silence_warnings do
          Encoding.default_external = value
          Encoding.default_internal = value
        end
      end

      def api_only=(value)

Review comment (suggestion):
@jeremy Sorry for my naive question. But how can someone mitigate uploads of big files, the analyze time directly in the request thread would be huge leading probably to timeouts. Thanks in advance

Code after:

          if respond_to?(:active_record)
            active_record.raise_on_missing_required_finder_order_columns = true
          end

          if respond_to?(:active_support)
            active_support.escape_js_separators_in_json = false
          end

          if respond_to?(:action_view)
            action_view.render_tracker = :ruby
          end

          if respond_to?(:action_view)
            action_view.remove_hidden_field_autocomplete = true
          end
        when "8.2"
          load_defaults "8.1"

          if respond_to?(:active_record)
            active_record.postgresql_adapter_decode_bytea = true
            active_record.postgresql_adapter_decode_money = true
          end

          if respond_to?(:active_storage)
            active_storage.analyze = :immediately
          end
        else
          raise "Unknown version #{target_version.to_s.inspect}"
        end

        @loaded_config_version = target_version
      end

      def reloading_enabled?
        enable_reloading
      end

      def enable_reloading
        !cache_classes
      end

      def enable_reloading=(value)
        self.cache_classes = !value
      end

      def encoding=(value)
        @encoding = value
        silence_warnings do
          Encoding.default_external = value
          Encoding.default_internal = value
Repository: rails/rails
File: activesupport/lib/active_support/combined_configuration.rb (Ruby)

Code before:
    #   option(:db_host)                             # => ENV["DB_HOST"] || Rails.app.credentials.option(:db_host)
    #   option(:database, :host)                     # => ENV["DATABASE__HOST"] || Rails.app.credentials.option(:database, :host)
    #   option(:database, :host, default: "missing") # => ENV["DATABASE__HOST"] || Rails.app.credentials.option(:database, :host) || "missing"
    #   option(:database, :host, default: -> { "missing" }) # => ENV["DATABASE__HOST"] || Rails.app.credentials.option(:database, :host) || "missing"
    def option(*key, default: nil)
      @configurations.each do |config|
        value = config.option(*key)
        return value unless value.nil?
      end

      default.respond_to?(:call) ? default.call : default
    end

    # Reload the cached values for all of the backend configurations.
    def reload
      @configurations.each { |config| config.try(:reload) }
    end

    def inspect # :nodoc:
      keys = @configurations.flat_map { |config|
        if config.is_a?(ActiveSupport::EnvConfiguration)
          config.instance_variable_get(:@envs).keys.map { |k| k.downcase.to_sym }
        elsif config.is_a?(ActiveSupport::EncryptedConfiguration)
          config.config.keys
        end
      }.compact.uniq
      "#<#{self.class.name}:#{'%#016x' % (object_id << 1)} keys=#{keys.inspect}>"
    end
  end
end


Review comment (suggestion):
Hmm, this is indeed rather messy. Probably fine to add #keys back just to provide this in a cleaner manner, so you can do `@configurations.flat_map(&:keys).uniq`.

Code after:
    #   option(:db_host)                             # => ENV["DB_HOST"] || Rails.app.credentials.option(:db_host)
    #   option(:database, :host)                     # => ENV["DATABASE__HOST"] || Rails.app.credentials.option(:database, :host)
    #   option(:database, :host, default: "missing") # => ENV["DATABASE__HOST"] || Rails.app.credentials.option(:database, :host) || "missing"
    #   option(:database, :host, default: -> { "missing" }) # => ENV["DATABASE__HOST"] || Rails.app.credentials.option(:database, :host) || "missing"
    def option(*key, default: nil)
      @configurations.each do |config|
        value = config.option(*key)
        return value unless value.nil?
      end

      default.respond_to?(:call) ? default.call : default
    end

    # Reload the cached values for all of the backend configurations.
    def reload
      @configurations.each { |config| config.try(:reload) }
    end

    # Returns a unique array of all symbolized keys available across all backend configurations.
    #
    # Examples of Rails-configured access:
    #
    #   ENV["DB_HOST"] = "localhost"
    #   Rails.app.credentials # has keys [:secret_key_base, :aws]
    #
    #   Rails.app.creds.keys
    #   # => [:db_host, :secret_key_base, :aws]
    #
    def keys
      @configurations.flat_map(&:keys).uniq
    end

    def inspect # :nodoc:
      "#<#{self.class.name}:#{'%#016x' % (object_id << 1)} keys=#{keys.inspect}>"
    end
  end
end
Repository: rails/rails
File: activesupport/lib/active_support/env_configuration.rb (Ruby)

Code before:
    #   option(:database, :host)                            # => ENV["DATABASE__HOST"]
    #   option(:database, :host, default: "missing")        # => ENV.fetch("DATABASE__HOST", "missing")
    #   option(:database, :host, default: -> { "missing" }) # => ENV.fetch("DATABASE__HOST") { default.call }
    def option(*key, default: nil)
      if default.respond_to?(:call)
        @envs.fetch(envify(*key)) { default.call }
      else
        @envs.fetch envify(*key), default
      end
    end

    # Reload the cached ENV values in case any of them changed or new ones were added during runtime.
    def reload
      @envs = ENV.to_h
    end

    # Returns an array of symbolized keys from the environment variables.
    #
    # Examples:
    #
    #   ENV["DB_HOST"] = "localhost"
    #   ENV["DATABASE__PORT"] = "5432"
    #
    #   env_config = ActiveSupport::EnvConfiguration.new
    #   env_config.keys
    #   # => [:db_host, :database__port, ...]
    #
    def keys
      @envs.keys.map { |k| k.downcase.to_sym }
    end

  def inspect # :nodoc:
    "#<#{self.class.name}:#{'%#016x' % (object_id << 1)} keys=#{keys.inspect}>"
  end

    private
      def lookup(env_key)
        @envs[env_key]
      end

      def envify(*key)
        key.collect { |part| part.to_s.upcase }.join("__")
      end
  end
end


Review comment (suggestion):
`:database__port` is not a valid key, actually. The keys are nested, so it would need to be presented as `database: [ :port ]` or the like. Not entirely trivial, but we can't present keys that aren't correct either.

Code after:
    #   option(:database, :host)                            # => ENV["DATABASE__HOST"]
    #   option(:database, :host, default: "missing")        # => ENV.fetch("DATABASE__HOST", "missing")
    #   option(:database, :host, default: -> { "missing" }) # => ENV.fetch("DATABASE__HOST") { default.call }
    def option(*key, default: nil)
      if default.respond_to?(:call)
        @envs.fetch(envify(*key)) { default.call }
      else
        @envs.fetch envify(*key), default
      end
    end

    # Reload the cached ENV values in case any of them changed or new ones were added during runtime.
    def reload
      @envs = ENV.to_h
    end

    # Returns an array of symbolized keys from the environment variables.
    #
    # Examples:
    #
    #   ENV["DB_HOST"] = "localhost"
    #   ENV["DATABASE_PORT"] = "5432"
    #
    #   env_config = ActiveSupport::EnvConfiguration.new
    #   env_config.keys
    #   # => [:db_host, :database_port, ...]
    #
    def keys
      @envs.keys.map { |k| k.downcase.to_sym }
    end

    def inspect # :nodoc:
      "#<#{self.class.name}:#{'%#016x' % (object_id << 1)} keys=#{keys.inspect}>"
    end

    private
      def lookup(env_key)
        @envs[env_key]
      end

      def envify(*key)
        key.collect { |part| part.to_s.upcase }.join("__")
      end
  end
end
Repository: rails/rails
File: tools/support/leak_checker.rb (Ruby)

Code before:
# frozen_string_literal: true

module LeakChecker
  # These keys are out of our control.
  # Either added by the system or by other libraries.
  ALLOWED_KEYS = %w[
    VIPSHOME
  ]

  def before_setup
    @@before_env = ENV.to_h
    super
  end

  def after_teardown
    super
    if (keys = ENV.keys - @@before_env.keys - ALLOWED_KEYS) && keys.any?
      flunk "Environment leak detected! #{keys.join(", ")}"
    end
  end
end


Review comment (suggestion):
I still think we should not only check the keys but also the values.

`if ENV.to_h != @@before_env then figure_out_the_diff`

Code after:
# frozen_string_literal: true

module LeakChecker
  # These keys are out of our control.
  # Either added by the system or by other libraries.
  ALLOWED_KEYS = %w[
    VIPSHOME
  ]

  def before_setup
    @__leak_checker_before_env = ENV.to_h
    super
  end

  def after_teardown
    super

    after = ENV.to_h
    before = @__leak_checker_before_env

    ALLOWED_KEYS.each do |k|
      after.delete(k)
      before.delete(k)
    end

    if after != before
      message = +"Environment leak detected:\n"
      added_keys = after.keys - before.keys
      unless added_keys.empty?
        message << "  - Added variables:\n"
        added_keys.each do |k|
          message << "    - #{k}"
        end
      end

      removed_keys = before.keys - after.keys
      unless removed_keys.empty?
        message << "  - Removed variables:\n"
        removed_keys.each do |k|
          message << "    - #{k}"
        end
      end
Repository: rails/rails
File: tools/support/leak_checker.rb (Ruby)

Code before:
# frozen_string_literal: true

module LeakChecker
  # These keys are out of our control.
  # Either added by the system or by other libraries.
  ALLOWED_KEYS = %w[
    VIPSHOME
  ]

  def before_setup
    @@before_env = ENV.to_h
    super
  end

  def after_teardown
    super
    if (keys = ENV.keys - @@before_env.keys - ALLOWED_KEYS) && keys.any?
      flunk "Environment leak detected! #{keys.join(", ")}"
    end
  end
end


Review comment (suggestion):
Not sure we should use class vars here. It's rare but technically you can "parallelize" tests with threads.

Code after:
# frozen_string_literal: true

module LeakChecker
  # These keys are out of our control.
  # Either added by the system or by other libraries.
  ALLOWED_KEYS = %w[
    VIPSHOME
  ]

  def before_setup
    @__leak_checker_before_env = ENV.to_h
    super
  end

  def after_teardown
    super

    after = ENV.to_h
    before = @__leak_checker_before_env

    ALLOWED_KEYS.each do |k|
      after.delete(k)
      before.delete(k)
    end

    if after != before
      message = +"Environment leak detected:\n"
      added_keys = after.keys - before.keys
      unless added_keys.empty?
        message << "  - Added variables:\n"
        added_keys.each do |k|
          message << "    - #{k}"
        end
      end

      removed_keys = before.keys - after.keys
Repository: rails/rails
File: actiontext/test/unit/attachable_test.rb (Ruby)

Code before:
    attributes = {
      id: nil,
      key: "123",
      filename: "test.txt",
      content_type: "text/plain",
      metadata: { identified: true },
      service_name: "test",
      byte_size: 4,
      checksum: "CY9rzUYh03PK3k6DJie09g==",
      created_at: nil,
      attachable_sgid: nil
    }.deep_stringify_keys

    assert_equal attributes, attachable.as_json
  end

  test "attachable_sgid is included in as_json when only option is nil or includes attachable_sgid" do
    attachable = ActiveStorage::Blob.create_after_unfurling!(io: StringIO.new("test"), filename: "test.txt", key: 123)

    assert_equal({ "id" => attachable.id }, attachable.as_json(only: :id))
    assert_equal({ "id" => attachable.id }, attachable.as_json(only: [:id]))
    assert_equal(attachable.as_json.except("attachable_sgid"), attachable.as_json(except: :attachable_sgid))
    assert_equal(attachable.as_json.except("attachable_sgid"), attachable.as_json(except: [:attachable_sgid]))
  end

  test "read_attribute_for_serialization returns the attribute " do
    attachable = ActiveStorage::Blob.create_after_unfurling!(io: StringIO.new("test"), filename: "test.txt", key: 123)

    assert_equal(attachable.key, attachable.read_attribute_for_serialization(:key))
  end
end


Review comment (suggestion):
```suggestion
  test "read_attribute_for_serialization returns the attribute" do
```

Code after:
    attributes = {
      id: nil,
      key: "123",
      filename: "test.txt",
      content_type: "text/plain",
      metadata: { identified: true },
      service_name: "test",
      byte_size: 4,
      checksum: "CY9rzUYh03PK3k6DJie09g==",
      created_at: nil,
      attachable_sgid: nil
    }.deep_stringify_keys

    assert_equal attributes, attachable.as_json
  end

  test "attachable_sgid is included in as_json when only option is nil or includes attachable_sgid" do
    attachable = ActiveStorage::Blob.create_after_unfurling!(io: StringIO.new("test"), filename: "test.txt", key: 123)

    assert_equal({ "id" => attachable.id }, attachable.as_json(only: :id))
    assert_equal({ "id" => attachable.id }, attachable.as_json(only: [:id]))
    assert_equal(attachable.as_json.except("attachable_sgid"), attachable.as_json(except: :attachable_sgid))
    assert_equal(attachable.as_json.except("attachable_sgid"), attachable.as_json(except: [:attachable_sgid]))
  end

  test "read_attribute_for_serialization returns the attribute" do
    attachable = ActiveStorage::Blob.create_after_unfurling!(io: StringIO.new("test"), filename: "test.txt", key: 123)

    assert_equal(attachable.key, attachable.read_attribute_for_serialization(:key))
  end
end
Repository: rails/rails
File: actiontext/lib/action_text/attachable.rb (Ruby)

Code before:
    #
    # Override to render a different partial:
    #
    #     class User < ApplicationRecord
    #       def to_attachable_partial_path
    #         "users/attachable"
    #       end
    #     end
    def to_attachable_partial_path
      to_partial_path
    end

    def to_rich_text_attributes(attributes = {})
      attributes.dup.tap do |attrs|
        attrs[:sgid] = attachable_sgid
        attrs[:content_type] = attachable_content_type
        attrs[:previewable] = true if previewable_attachable?
        attrs[:filename] = attachable_filename
        attrs[:filesize] = attachable_filesize
        attrs[:width] = attachable_metadata[:width]
        attrs[:height] = attachable_metadata[:height]
      end.compact
    end

    def read_attribute_for_serialization(key)
      if key == "attachable_sgid"
        persisted? ? super : nil
      else
        super
      end
    end

    private
      def attribute_names_for_serialization
        super + ["attachable_sgid"]
      end
  end
end


Review comment (suggestion):
Maybe link to the ActiveModel documentation as well?
```suggestion
    # See ActiveModel::Serialization#read_attribute_for_serialization.
    def read_attribute_for_serialization(key)
```

Code after:
    #
    # Override to render a different partial:
    #
    #     class User < ApplicationRecord
    #       def to_attachable_partial_path
    #         "users/attachable"
    #       end
    #     end
    def to_attachable_partial_path
      to_partial_path
    end

    def to_rich_text_attributes(attributes = {})
      attributes.dup.tap do |attrs|
        attrs[:sgid] = attachable_sgid
        attrs[:content_type] = attachable_content_type
        attrs[:previewable] = true if previewable_attachable?
        attrs[:filename] = attachable_filename
        attrs[:filesize] = attachable_filesize
        attrs[:width] = attachable_metadata[:width]
        attrs[:height] = attachable_metadata[:height]
      end.compact
    end

    # See ActiveModel::Serialization#read_attribute_for_serialization.
    def read_attribute_for_serialization(key)
      if key == "attachable_sgid"
        persisted? ? super : nil
      else
        super
      end
    end

    private
      def attribute_names_for_serialization
        super + ["attachable_sgid"]
      end
  end
end
Repository: rails/rails
File: activerecord/lib/active_record/query_logs.rb (Ruby)

Code before:

        def uncached_comment(**options)
          content = tag_content(**options)

          if content.present?
            "/*#{escape_sql_comment(content)}*/"
          end
        end

        def escape_sql_comment(content)
          # Sanitize a string to appear within an SQL comment
          # For compatibility, this also surrounding "/*+", "/*", and "*/"
          # characters, possibly with single surrounding space.
          # Then follows that by replacing any internal "*/" or "/ *" with
          # "* /" or "/ *"
          comment = content.to_s.dup
          comment.gsub!(%r{\A\s*/\*\+?\s?|\s?\*/\s*\Z}, "")
          comment.gsub!("*/", "* /")
          comment.gsub!("/*", "/ *")
          comment
        end

        def tag_content(**options)
          context = ActiveSupport::ExecutionContext.to_h
          context[:sql] = options[:sql]
          context[:connection] ||= options[:connection]

          pairs = @handlers.filter_map do |(key, handler)|
            val = handler.call(context)
            @formatter.format(key, val) unless val.nil?
          end
          @formatter.join(pairs)
        end
    end

    @handlers = rebuild_handlers
    self.tags_formatter = :legacy
  end
end


Review comment (suggestion):
```suggestion
          context.merge!(options)
```

Code after:
        end

        # Returns an SQL comment +String+ containing the query log tags.
        # Sets and returns a cached comment if <tt>cache_query_log_tags</tt> is +true+.
        def comment(extra_context)
          if cache_query_log_tags
            self.cached_comment ||= uncached_comment(extra_context)
          else
            uncached_comment(extra_context)
          end
        end

        def uncached_comment(extra_context)
          content = tag_content(extra_context)

          if content.present?
            "/*#{escape_sql_comment(content)}*/"
          end
        end

        def escape_sql_comment(content)
          # Sanitize a string to appear within an SQL comment
          # For compatibility, this also surrounding "/*+", "/*", and "*/"
          # characters, possibly with single surrounding space.
          # Then follows that by replacing any internal "*/" or "/ *" with
          # "* /" or "/ *"
          comment = content.to_s.dup
          comment.gsub!(%r{\A\s*/\*\+?\s?|\s?\*/\s*\Z}, "")
          comment.gsub!("*/", "* /")
          comment.gsub!("/*", "/ *")
          comment
        end

        def tag_content(extra_context)
          context = ActiveSupport::ExecutionContext.to_h
          context.reverse_merge!(extra_context)

          pairs = @handlers.filter_map do |(key, handler)|
            val = handler.call(context)
            @formatter.format(key, val) unless val.nil?
          end
          @formatter.join(pairs)
        end
    end

    @handlers = rebuild_handlers
    self.tags_formatter = :legacy
  end
end
Repository: rails/rails
File: activestorage/test/models/blob_test.rb (Ruby)

Code before:
    assert_predicate blob, :image?
    assert_not_predicate blob, :audio?
  end

  test "video?" do
    blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4")
    assert_predicate blob, :video?
    assert_not_predicate blob, :audio?
  end

  test "text?" do
    blob = create_blob data: "Hello world!"
    assert_predicate blob, :text?
    assert_not_predicate blob, :audio?
  end

  test "blob type methods return false for nil content type" do
    blob = create_blob_before_direct_upload(
      filename: "unknown_file",
      byte_size: 100,
      checksum: "test_checksum",
      content_type: nil
    )

    assert_nil blob.content_type
    assert_equal false, blob.image?
    assert_equal false, blob.video?
    assert_equal false, blob.audio?
    assert_equal false, blob.text?
  end

  test "download yields chunks" do
    blob   = create_blob data: "a" * 5.0625.megabytes
    chunks = []

    blob.download do |chunk|
      chunks << chunk
    end

    assert_equal 2, chunks.size
    assert_equal "a" * 5.megabytes, chunks.first
    assert_equal "a" * 64.kilobytes, chunks.second
  end

  test "open yielding with integrity" do
    create_file_blob(filename: "racecar.jpg").tap do |blob|
      blob.open do |file|
        assert_predicate file, :binmode?
        assert_equal 0, file.pos
        assert File.basename(file.path).start_with?("ActiveStorage-#{blob.id}-")
        assert file.path.end_with?(".jpg")

Review comment (suggestion):
You can use the assert_not_predicate instead:
```suggestion
    assert_not_predicate blob, :image?
```

Code after:
    assert_predicate blob, :image?
    assert_not_predicate blob, :audio?
  end

  test "video?" do
    blob = create_file_blob(filename: "video.mp4", content_type: "video/mp4")
    assert_predicate blob, :video?
    assert_not_predicate blob, :audio?
  end

  test "text?" do
    blob = create_blob data: "Hello world!"
    assert_predicate blob, :text?
    assert_not_predicate blob, :audio?
  end

  test "blob type methods return false for nil content type" do
    blob = create_blob_before_direct_upload(
      filename: "unknown_file",
      byte_size: 100,
      checksum: "test_checksum",
      content_type: nil
    )

    assert_nil blob.content_type
    assert_not_predicate blob, :image?
    assert_not_predicate blob, :video?
    assert_not_predicate blob, :audio?
    assert_not_predicate blob, :text?
  end

  test "download yields chunks" do
    blob   = create_blob data: "a" * 5.0625.megabytes
    chunks = []

    blob.download do |chunk|
      chunks << chunk
    end

    assert_equal 2, chunks.size
    assert_equal "a" * 5.megabytes, chunks.first
    assert_equal "a" * 64.kilobytes, chunks.second
  end

  test "open yielding with integrity" do
    create_file_blob(filename: "racecar.jpg").tap do |blob|
      blob.open do |file|
        assert_predicate file, :binmode?
        assert_equal 0, file.pos
        assert File.basename(file.path).start_with?("ActiveStorage-#{blob.id}-")
        assert file.path.end_with?(".jpg")