Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Rails Record getting saved even if errors got set

Here is my credential model:

class Credential < ApplicationRecord
  validate :password_or_certificate
  enum credential_type: { windows: 1, linux: 2 }

  def password_or_certificate
    unless user_pass.blank? ^ cert_file_path.blank?
      errors.add(:base, "Please provide a password or a certificate, not both.")
    end
  end
end

I am checking for the certificate in the controller and setting the error in controller like this:

def create
    attachment = params[:credential][:cert_file_path]
    cert_file_path = Rails.root.join('private', 'certificates', attachment.original_filename) if attachment.present?

    @credential = Credential.new(credential_params)
    @credential.cert_file_path = cert_file_path

    if @credential.valid? && cert_file_path.present?
      cert_error_msg = 'Certificate is not valid'
      fm = FileMagic.new(FileMagic::MAGIC_MIME)
      file_path = attachment.path
      if fm.file(file_path) =~ /^text\//
        puts first_line = File.open(attachment.path) { |f| f.readline }
        if first_line.include? '-----BEGIN RSA PRIVATE KEY-----'
          File.open(cert_file_path, 'w') { |f| f.write(attachment.read) }
        else
          @credential.errors.add(:cert_file_path, cert_error_msg)
        end
      else
        @credential.errors.add(:cert_file_path, cert_error_msg)
      end
    end

    respond_to do |format|
      if @credential.save
        format.html { redirect_to credentials_url, notice: 'Credential was successfully mapped.' }
        format.js
        format.json { render :show, status: :created, location: @credential }
      else
        format.html { render :new }
        format.js
        format.json { render json: @credential.errors, status: :unprocessable_entity }
      end
    end
  end

Even though errors are getting set, the record is getting saved.

#<ActiveModel::Errors:0x0000557781babde8 @base=#<Credential id: nil, alias: "My new credential", user_name: "raj", encrypted_user_pass: "", encrypted_user_pass_iv: "VrT0xsxYtf//cwVx\n", credential_type: "linux", created_at: nil, updated_at: nil, cert_file_path: "/home/rmishra/awsapp/private/certificates/Ruby Enc...", passphrase: "">, @messages={:cert_file_path=>["Certificate is not valid"]}, @details={:cert_file_path=>[{:error=>"Certificate is not valid"}]}>

I know I can check for @credential.errors.blank? and then save it, can you guys help me out to put all these logic in my model.

like image 373
Rajkaran Mishra Avatar asked Jan 09 '19 07:01

Rajkaran Mishra


3 Answers

I think you expected ActiveRecord to not save record if you manually set up the error on it.

But it's not.

ActiveRecord on save command starts its validations, when one of them is not fit then AR sets the error and return false on save

In your code there's no validation for cert_file_path field so it perfectly suits to be saved.

The right way is to move the validation of cert_file_path from controller to Credential, on save event it will call that validation, if it is not fit a validation it will set the corresponding error up and return false on save.

like image 137
megas Avatar answered Nov 20 '22 17:11

megas


You can create a custom validation method to move that logic to your model, and create virtual attribute to get the access to the attachment object

Model

class Credential < ApplicationRecord
  validate :password_or_certificate
  enum credential_type: { windows: 1, linux: 2 }

  # create a virtual attribute to store attachment
  attr_accessor :attachment

  # Custom validation method on ActiveRecord object creation
  validate :create_certificate, on: :create

  def create_certificate
    if cert_file_path.present?
      cert_error_msg = 'Certificate is not valid'
      fm = FileMagic.new(FileMagic::MAGIC_MIME)
      file_path = attachment.path
      if fm.file(file_path) =~ /^text\//
        puts first_line = File.open(attachment.path) {|f| f.readline}
        if first_line.include? '-----BEGIN RSA PRIVATE KEY-----'
          File.open(cert_file_path, 'w') {|f| f.write(attachment.read)}
        else
          errors.add(:cert_file_path, cert_error_msg)
        end
      else
        errors.add(:cert_file_path, cert_error_msg)
      end
    else
      errors.add(:cert_file_path, cert_error_msg)
    end
  end

  def password_or_certificate
    unless user_pass.blank? ^ cert_file_path.blank?
      errors.add(:base, "Please provide a password or a certificate, not both.")
    end
  end
end

Controller

def create
  attachment = params[:credential][:cert_file_path]
  cert_file_path = Rails.root.join('private', 'certificates', attachment.original_filename) if attachment.present?

  @credential = Credential.new(credential_params)
  @credential.cert_file_path = cert_file_path
  @credential.attachment = attachment

  respond_to do |format|
    if @credential.save
      format.html {redirect_to credentials_url, notice: 'Credential was successfully mapped.'}
      format.js
      format.json {render :show, status: :created, location: @credential}
    else
      format.html {render :new}
      format.js
      format.json {render json: @credential.errors, status: :unprocessable_entity}
    end
  end
end
like image 27
Javier Menéndez Rizo Avatar answered Nov 20 '22 15:11

Javier Menéndez Rizo


When Rails save a record, it invokes internal validate method, the method clears the error message you set before running model validation code. so adding error message outside validation can not prevent record from saving. In your case, write this

if @credential.errors.empty? && @credential.save

But I strongly advise you write validation logic in validators

like image 39
Pengcheng Zhou Avatar answered Nov 20 '22 15:11

Pengcheng Zhou