fix: dont retrieve from store until accessing file

Author: Amnesthesia

lib/carrierwave/mounter.rb

@@ -66,7 +66,11 @@ def read_identifiers
     def uploaders
       @uploaders ||= read_identifiers.map do |identifier|
         uploader = blank_uploader
-        uploader.retrieve_from_store!(identifier)
+
+        # Assign the identifier to the uploader here,
+        # instead of forcing the uploader to make a round-trip
+        # to the store if we already have the identifier.
+        uploader.identifier = identifier
         uploader
       end
     end

lib/carrierwave/uploader.rb

@@ -42,8 +42,6 @@ module Uploader
     # be trivial.
     #
     class Base
-      attr_reader :file
-
       include CarrierWave::Uploader::Configuration
       include CarrierWave::Uploader::Callbacks
       include CarrierWave::Uploader::Proxy

lib/carrierwave/uploader/configuration.rb

@@ -214,7 +214,7 @@ def reset_config
             config.base_path = CarrierWave.base_path
             config.enable_processing = true
             config.ensure_multipart_form = true
-            config.download_retry_count = 0
+            config.download_retry_count = 3
             config.download_retry_wait_time = 5
           end
         end

lib/carrierwave/uploader/mountable.rb

@@ -3,6 +3,7 @@ module Uploader
     module Mountable
 
       attr_reader :model, :mounted_as
+      attr_writer :identifier
 
       ##
       # If a model is given as the first parameter, it will be stored in the

lib/carrierwave/uploader/remove.rb

@@ -10,7 +10,7 @@ module Remove
       #
       def remove!
         with_callbacks(:remove) do
-          @file.delete if @file
+          @file.delete if file
           @file = nil
           @cache_id = nil
         end

lib/carrierwave/uploader/store.rb

@@ -8,6 +8,8 @@ module Store
       include CarrierWave::Uploader::Cache
 
       included do
+        attr_accessor :retrieval_retry_count
+
         prepend Module.new {
           def initialize(*)
             super
@@ -93,17 +95,41 @@ def store!(new_file=nil)
         end
       end
 
+      # Attempt to retrieve the file from the store
+      # if the file is nil, but to avoid calling this
+      # over and over if retrieving it returns nil,
+      # we keep track of the number of times we've
+      # attempted to retrieve it and just return nil
+      # if we exceed the maximum number of retries
+      #
+      # === Returns
+      #
+      # [CarrierWave::SanitizedFile, nil] the stored file or nil
+      def file
+        return @file unless @identifier
+        retrieve_from_store!(@identifier) unless @file
+        @file
+      end
+
       ##
       # Retrieves the file from the storage.
       #
       # === Parameters
       #
       # [identifier (String)] uniquely identifies the file to retrieve
       #
-      def retrieve_from_store!(identifier)
-        with_callbacks(:retrieve_from_store, identifier) do
-          @file = storage.retrieve!(identifier)
-          @identifier = identifier
+      def retrieve_from_store!(file_identifier)
+        self.retrieval_retry_count ||= 0
+        return if self.retrieval_retry_count > download_retry_count
+        self.retrieval_retry_count += 1
+        with_callbacks(:retrieve_from_store, file_identifier) do
+          # We can't retrieve the file if we have no identifier
+          # to retrieve it with. Identifier should be assigned
+          # when setting up the uploader, or when caching,
+          # or storing a new file
+          next unless file_identifier
+          @file = storage.retrieve!(file_identifier)
+          @identifier = file_identifier
         end
       end
 

spec/spec_helper.rb

@@ -162,6 +162,7 @@ def stub_request(method, uri)
   config.around :each, :with_retry do |example|
     example.run_with_retry retry: 2
   end
+  config.example_status_persistence_file_path = "tmp/failures.txt"
   config.retry_callback = proc do |example|
     sleep 1
   end