Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using audited in Rails 7.0.7 generates a lot of deprecation warnings #682

Closed
vrinek opened this issue Aug 29, 2023 · 3 comments
Closed

Using audited in Rails 7.0.7 generates a lot of deprecation warnings #682

vrinek opened this issue Aug 29, 2023 · 3 comments

Comments

@vrinek
Copy link

vrinek commented Aug 29, 2023

This is an example of the warnings:

DEPRECATION WARNING: Using a :default format for Date#to_s is deprecated. Please use Date#to_fs instead. (called from block (4 levels) in <top (required)> at /app/spec/factories/user.rb:16)

Removing the audited line from the User model silences these warnings which leads me to think that the root cause might lie within this gem.

From Rails' side, these warnings started being emitted since this commit: rails/rails@7e9ffc2 (in the 7.0.7 release, August 9th, see https://github.com/rails/rails/blob/7-0-stable/activesupport/CHANGELOG.md#rails-707-august-09-2023).

Any tips to help pinpoint where these deprecation warnings originate from exactly would be much appreciated.

@BrianHawley
Copy link

BrianHawley commented Sep 1, 2023

@vrinek,

This happens because Psych (the YAML serializer bundled with Ruby) serializes Date values with to_s, and just assumes that to_s generates ISO-8601 format dates. This is a little presumptuous of Psych, in my opinion.

Worse, if you override Date::DATE_FORMATS[:default] in your codebase to match another format - probably not recommended - then this breaks YAML serialization for Rails versions before they fix that deprecation (hopefully in Rails 7.1) by generating data in the Date::DATE_FORMATS[:default] format. This will make such values deserialize as strings rather than Date values.

Here's an initializer patch for both issues, for a codebase where someone changed Date::DATE_FORMATS[:default] to the USA date format ("%m/%d/%Y"). If you didn't override Date::DATE_FORMATS[:default] you won't need the second patch, and you probably don't need the unless clause around the first patch (because it detects the format change, not the warning). Pardon the rubocop and reek pragma comments.

# frozen_string_literal: true

# NOTE: Overriding Date::DATE_FORMATS[:default] breaks YAML serialization of Date values.

date = Date.today

# Check and patch YAML serialization of Date values, if necessary.
unless ActiveSupport::Deprecation.silence { YAML.dump(date) } == "--- #{date.strftime('%F')}\n"
  # Override format and apply https://github.com/ruby/psych/pull/573 too.
  Psych::Visitors::YAMLTree.class_exec do
    # :reek:UncommunicativeMethodName and :reek:UncommunicativeParameterName are irrelevant here.
    def visit_Date(o) # rubocop:disable Naming/MethodName
      formatted = o.gregorian.strftime("%F")
      register(o, @emitter.scalar(formatted, nil, nil, true, false, ::Psych::Nodes::Scalar::ANY))
    end
  end
end

# Check YAML deserialization of the old overriden format, and patch if necessary.
unless YAML.unsafe_load("--- #{date.strftime('%m/%d/%Y')}\n") == date
  # Parse the Date strings that we used to generate before the above patch.
  Psych::ScalarScanner.prepend(
    Module.new do
      def tokenize(string)
        return nil if string.empty?

        if string.match?(/^(?:1[012]|0\d|\d)\/(?:[12]\d|3[01]|0\d|\d)\/\d{4}$/)
          # US format date
          require "date"
          begin
            class_loader.date.strptime(string, "%m/%d/%Y", ::Date::GREGORIAN)
          rescue ArgumentError
            string
          end
        else
          super
        end
      end
    end
  )
end

@BrianHawley
Copy link

Reported to Psych upstream: ruby/psych#644

@vrinek
Copy link
Author

vrinek commented Sep 4, 2023

Oof, I was worried it would be Psych. Thanks for pointing it out and providing a possible workaround. I'll close this issue now since there's nothing to do in audited to fix this.

@vrinek vrinek closed this as completed Sep 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants