Bullet and ActiveStore and ActiveStorageDB intersection?

[epugh]

Hi, I'm looking at upgrading my use of Bullet from 8.0.1 to 8.0.7, and I'm now getting a failure from Bullet, which I think is false, but not totally sure, so wanted to run by you, because I am curious why Bullet might think this is eager loading?

Here is the error:

Bullet::Notification::UnoptimizedQueryError: user: circleci...
Failure:
test_should_import_a_valid_ZIP_file_containing_a_JSON_file(Minitest::Result) [/home/circleci/project/vendor/bundle/ruby/3.4.0/gems/uniform_notifier-1.17.0/lib/uniform_notifier/raise.rb:19]:
Bullet::Notification::UnoptimizedQueryError: user: circleci
POST /books/import
AVOID eager loading detected
  ActiveStorage::Attachment => [:record]
  Remove from your query: .includes([:record])
Call stack
  /home/circleci/project/app/controllers/books/import_controller.rb:64:in 'Books::ImportController#create'
  /home/circleci/project/test/controllers/books/import_controller_test.rb:49:in 'block in <class:BooksControllerTest>'


    test/controllers/books/import_controller_test.rb:49:in 'block in <class:BooksControllerTest>'

and line 64 is the @book.import_file.attach... Is there a better/more optimized way I should be doing this?

if @book.errors.empty? && @book.save
        serialized_data = Marshal.dump(params_to_use)
        compressed_data = Zlib::Deflate.deflate(serialized_data)
        @book.import_file.attach(io: StringIO.new(compressed_data), filename: "book_import_#{@book.id}.bin.zip",
                                 content_type: 'application/zip')
        @book.save

        track_book_import_queued do
          ImportBookJob.perform_later current_user, @book
        end
        redirect_to @book, notice: 'Book was successfully created.'
      else
        render :new
      end

For reference, here is the line in the main branch: https://github.com/o19s/quepid/blob/main/app/controllers/books/import_controller.rb#L64

[blocknotes]

Hey @epugh,
thanks for reporting.

I'm inspecting this point.
I have just added bullet: #70

And there's a failing spec: #72
spec/integration/attachments_spec.rb#15 with the same offense:

     Bullet::Notification::UnoptimizedQueryError:
       user: app
        
       AVOID eager loading detected
         ActiveStorage::Attachment => [:record]
         Remove from your query: .includes([:record])

For now I can tell you little, but we'll see.

[blocknotes]

@epugh, from my understanding the offense is outside active_storage_db

It seems related directly to active_storage itself.

I think that the specific includes in the issue should be here:
https://github.com/rails/rails/blob/main/activestorage/app/models/active_storage/blob.rb#L366
The record association is defined here:
https://github.com/rails/rails/blob/main/activestorage/app/models/active_storage/attachment.rb#L25

Checking the code you can avoid the touch operation:
ActiveStorage.touch_attachment_records = false
With this option the failing test works.

But you'll need to evaluate if the consequences of disabling it could be an impact in your app/component.

Closing the issue, but please reopen it if this is not the root cause.

[epugh]

Thanks for the analysis! Is this potentially a fix in active_storage itself that was done a few weeks ago? rails/rails#54921.