Memoize key_provider from key or deterministic key_provider if any

[rosa]

Motivation / Background

We're in the process of bringing one of our apps up to date with main, and in the process of doing so, we realised our tests were taking about 5 times longer to run. We traced the slowdown back to #51019.

The memoization of Scheme#key_provider was removed completely in that PR because it prevented overriding the key_provider via with_encryption_context. For encrypted attributes where we either provide a key or declare them as deterministic, we have to derive a key to instantiate the provider every time we load them. This might happen hundreds of times per test, and ultimately means we call

ActiveSupport::KeyGenerator#generate_key

and

OpenSSL::KDF.pbkdf2_hmac

hundreds of times. This adds significant overhead per test.

In reality, what's overridden by with_encryption_context is the value used as default provider, that is, ActiveRecord::Encryption.key_provider. This is only used in Scheme#key_provider if the scheme doesn't already have either a key_provider passed directly, or a key, or is deterministic, because it's called via default_key_provider here:

rails/activerecord/lib/active_record/encryption/scheme.rb

Lines 52 to 54 in 3efae44

	def key_provider
	@key_provider_param || build_key_provider || default_key_provider
	end

rails/activerecord/lib/active_record/encryption/scheme.rb

Lines 91 to 93 in 3efae44

	def default_key_provider
	ActiveRecord::Encryption.key_provider
	end

However, build_key_provider takes precedence, and it's very expensive to do:

rails/activerecord/lib/active_record/encryption/scheme.rb

Lines 83 to 89 in 3efae44

	def build_key_provider
	return DerivedSecretKeyProvider.new(@key) if @key.present?
	if @deterministic
	DeterministicKeyProvider.new(ActiveRecord::Encryption.config.deterministic_key)
	end
	end

Neither @key nor @deterministic can be overridden, so this PR memoizes them if we have to build them.

Checklist

Before submitting the PR make sure the following are checked:

• This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
• Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
• Tests are added or updated if you fix a bug or add a feature.
• CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

cc @jhawthorn @kstevens715

[jhawthorn]

Neither @key nor @deterministic can be overridden, so this PR memoizes them if we have to build them.

👍 If that's true this all makes sense to me. Let's make sure to backport this to 7-1-stable as well (as the previous fix was)

[jhawthorn]

Thanks Rosa! Also merged into 7-1-stable in 71bb84b

[jorgemanrubia]

Amazing work here @rosa 👏

[rosa]

Yay! Thank you so much, @jhawthorn and @jorgemanrubia ❤️ 🙏