Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Removing hard dependency on ActiveModel for BCrypt as per issue #2687 #2754

Closed
wants to merge 1 commit into from

4 participants

Jared McFarland Aaron Patterson Sjoerd Tieleman Santiago Pastorino
Aaron Patterson
Owner

I don't think this is complete. It still needs this:

  • dependency removed from the gemspec
  • A call to gem to activate the correct version (~> 3.0.0)
  • A call to require to require it
  • The previous two must only be done when someone uses has_secure_password
Jared McFarland

Is this an acceptable way of doing the last two?

def has_secure_password
  gem 'bcrypt-ruby', '~> 3.0.0'
  require 'bcrypt'

Also, why does it need to be required if you use autoload? Won't autoload call require the first time you try to access bcrypt-ruby?

Sjoerd Tieleman

Might want to wrap that in a begin/rescue block? Like:

def has_secure_password
  begin
    gem 'bcrypt-ruby', '~> 3.0.0'
    require 'bcrypt'
  rescue LoadError
    # give meaningful error about bcrypt
  end

Looks like ActiveRecord::Base includes this file by default and it is always loaded, so we can't put it at the top of the file. My first idea was to put it in has_secure_password as well.

Jared McFarland

The following is now causing the tests to fail with the error message I used:

      def has_secure_password
        begin
          gem 'bcrypt-ruby', '~> 3.0.0'
          require 'bcrypt'
        rescue LoadError
          raise "The bcrypt-ruby gem ~> 3.0.0 is required to use ActiveModel#has_secure_password"
        end

But this doesn't cause any errors in irb

[jared@Adam-Smith:~/gems/rails-1/activemodel (bcrypt-hard-dependency)]$ irb
ruby-1.9.2-p290 :001 > gem 'bcrypt-ruby', '~> 3.0.0'
 => true 
ruby-1.9.2-p290 :002 > require 'bcrypt'
 => true 
Santiago Pastorino
Owner

This is already fixed

Santiago Pastorino spastorino closed this
Santiago Pastorino
Owner

This is already fixed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  activemodel/lib/active_model/secure_password.rb
2  activemodel/lib/active_model/secure_password.rb
View
@@ -1,4 +1,4 @@
-require 'bcrypt'
+autoload :BCrypt, 'bcrypt'
module ActiveModel
module SecurePassword
Something went wrong with that request. Please try again.