Cannot create model with attached activestorage file with strict_loading set to true

Steps to reproduce

Create a simple model with

has_one_attached :profile, strict_loading: true

Then create the model with a such a profile (file).

An ActiveRecord::StrictLoadingViolationError is raised while the model is being created.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  git_source(:github) { |repo| "https://github.com/#{repo}.git" }

  # Activate the gem you are reporting the issue against.
  gem "rails", "~> 7.0.0"
  gem "sqlite3"
end

require "active_record/railtie"
require "active_storage/engine"
require "tmpdir"

class TestApp < Rails::Application
  config.root = __dir__
  config.hosts << "example.org"
  config.eager_load = false
  config.session_store :cookie_store, key: "cookie_store_key"
  secrets.secret_key_base = "secret_key_base"

  config.logger = Logger.new($stdout)
  Rails.logger  = config.logger

  config.active_storage.service = :local
  config.active_storage.service_configurations = {
    local: {
      root: Dir.tmpdir,
      service: "Disk"
    }
  }
end

ENV["DATABASE_URL"] = "sqlite3::memory:"

Rails.application.initialize!

require ActiveStorage::Engine.root.join("db/migrate/20170806125915_create_active_storage_tables.rb").to_s

ActiveRecord::Schema.define do
  CreateActiveStorageTables.new.change

  create_table :users, force: true
end

class User < ActiveRecord::Base
  has_one_attached :profile, strict_loading: true
end

require "minitest/autorun"

class BugTest < Minitest::Test
  def test_upload_and_download
    user = User.create!(
      profile: {
        content_type: "text/plain",
        filename: "dummy.txt",
        io: ::StringIO.new("dummy"),
      }
    )

    assert_equal "dummy", user.profile.download
  end
end

Expected behavior

The strict_loading validation should not occur on creating the record, only on reading it.

Actual behavior

An ActiveRecord::StrictLoadingViolationError is raised.

System configuration

Rails 7.0.2.3:

Ruby 3.1.2:

Hi! I have been digging into this issue, and I found that it happens when we initialize a model class sending the file as an argument. e.g.

class Admin < User
  has_one_attached :avatar, strict_loading: true
end

user = Admin.new(name: "Jason", avatar: create_blob(filename: "funky.jpg"))
user.save!

# ActiveRecord::StrictLoadingViolationError

The rule is being applied even if it is initialization and not a query. I understand that strict loading prevents the user from accessing lazy-loaded associations of a query result. So it doesn't make sense to avoid lazy loading when creating or initializing a model. I think that a good solution would be to check in the ActiveRecord::Associations::Association#violates_strict_loading? if the owner was previously a new record previously_new_record? for ignoring the rule in that case. it can be something like this =>

# activerecord/lib/active_record/associations/association.rb
def violates_strict_loading?
  return if owner.previously_new_record?
  return reflection.strict_loading? if reflection.options.key?(:strict_loading)

  owner.strict_loading? && !owner.strict_loading_n_plus_one_only?
end

It's just a thought. I tested this in my local machine, and all the ActiveStorage tests passed, and also the issue got fixed, but I'm not sure if this can break other parts of the code 🤔

Hey @neriojnavea ! I'd also been digging into this a bit, but had shelved it to focus on some other things.

So it doesn't make sense to avoid lazy loading when creating or initializing a model

I think a problem here is that we not only want to avoid the strict loading error when initializing a model with an attachment, but also when using the #attach API. For example:

class Admin < User
  has_one_attached :avatar, strict_loading: true
end

user = Admin.new(name: "Jason")
user.avatar.attach(create_blob) # This currently raises ActiveRecord::StrictLoadingViolationError

In which case, checking whether the attachment owner is a new record doesn't help us.

I think we need a way to only check for strict loading violations when a model's attachment(s) are loaded. I haven't found the right spot for this yet though 🤔

This issue has been automatically marked as stale because it has not been commented on for at least three months. The resources of the Rails team are limited, and so we are asking for your help. If you can still reproduce this error on the 7-0-stable branch or on main, please reply with all of the information you have about it in order to keep the issue open. Thank you for all your contributions.

Would you mean?

user = Admin.create(name: “Jason”) user.avatar.attach(create_blob)

to clarify that there should not be any check on strict_loading when adding an attachment on an existing record (parent) ?

Op 1 jun. 2022, om 17:18 heeft Adrianna Chang @.***> het volgende geschreven:

Hey @neriojnavea https://github.com/neriojnavea ! I'd also been digging into this a bit, but had shelved it to focus on some other things.

So it doesn't make sense to avoid lazy loading when creating or initializing a model

I think a problem here is that we not only want to avoid the strict loading error when initializing a model with an attachment, but also when using the #attach https://edgeapi.rubyonrails.org/classes/ActiveStorage/Attached/One.html#method-i-attach API. For example:

class Admin < User has_one_attached :avatar, strict_loading: true end

user = Admin.new(name: "Jason") user.avatar.attach(create_blob) # This currently raises ActiveRecord::StrictLoadingViolationError In which case, checking whether the attachment owner is a new record doesn't help us.

I think we need a way to only check for strict loading violations when a model's attachment(s) are loaded. I haven't found the right spot for this yet though 🤔

— Reply to this email directly, view it on GitHub https://github.com/rails/rails/issues/44946#issuecomment-1143745202, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACY2N5VLUDLTLJO4VKNSE3VM55MXANCNFSM5UFGMLOA. You are receiving this because you authored the thread.