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

Idea: Don't return virtual attributes in attribute methods unless the ciphertext attribute is present #186

Open
4ndypanda opened this issue Jan 16, 2024 · 2 comments

Comments

@4ndypanda
Copy link
Contributor

I was also annoyed by this issue since I constantly run into ActiveModel::MissingAttributeError when doing stuff like

# User class defined in test/support/active_record.rb
User.create!(email: "test@example.org")
user = User.select('id').last

# Normally this is done with some metaprogramming, just showing an example
# that raises ActiveModel::MissingAttributeError
user.email if user.has_attribute?("email")

What are your thoughts on overriding #init_with_attributes in ActiveRecord::Core? Before ever setting the @attributes instance variable for the model, we could filter and check if the ciphertext attribute is present

# From ActiveRecord::Core
def init_with_attribute(attributes, new_record = false)
  # filter out lockbox attributes that don't have ciphertext attribute loaded
  attributes = attributes.reject do |attribute, _value|
    self.lockbox_attributes[attribute.to_sym].present? &&
    !attributes.key?(
      self.lockbox_attributes[attribute.to_sym][:encrypted_attribute],
    )
  end
  super(attributes, new_record)
end

I was trying to see where else to do it. The caller here takes the attributes from the database result set and merges it with the self.class._default_attributes that includes the virtual attributes.

# Given a class, an attributes hash, +instantiate_instance_of+ returns a
# new instance of the class. Accepts only keys as strings.
def instantiate_instance_of(klass, attributes, column_types = {}, &block)
  attributes = klass.attributes_builder.build_from_database(attributes, column_types)
  klass.allocate.init_with_attributes(attributes, &block)
end

Overriding attributes_builder.build_from_database (source) is another option.

What are your thoughts?

@4ndypanda
Copy link
Contributor Author

A tricky thing is if people do want virtual attributes with a ciphertext virtual attribute (not from an ActiveRecord column)

class User < ActiveRecord::Base
  # This is never saved to the database, idk why one
  # would want this though.
  attribute :foo_ciphertext
  has_encrypted :foo
end

This is one case where it probably should still show up in the attribute methods attributes, attribute_names, has_attribute?.

@ankane
Copy link
Owner

ankane commented Jan 18, 2024

Hey @4ndypanda, will take a look at this as part of 2.0 since it's a breaking change.

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