Use traits to indicate failure/success

Instead of setting success: true/false manually, use the traits, to
ensure all the corresponding attributes are set correctly too.

ee/spec/finders/geo/attachment_registry_finder_spec.rb

@@ -87,7 +87,7 @@ describe Geo::AttachmentRegistryFinder, :geo do
       end
 
       it 'counts attachments that have been synced' do
-        create(:geo_file_registry, :attachment, file_id: upload_1.id, success: false)
+        create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
         create(:geo_file_registry, :attachment, file_id: upload_2.id)
         create(:geo_file_registry, :attachment, file_id: upload_3.id)
 
@@ -114,7 +114,7 @@ describe Geo::AttachmentRegistryFinder, :geo do
         end
 
         it 'counts attachments that has been synced' do
-          create(:geo_file_registry, :attachment, file_id: upload_1.id, success: false)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
           create(:geo_file_registry, :attachment, file_id: upload_2.id)
           create(:geo_file_registry, :attachment, file_id: upload_3.id)
 
@@ -142,7 +142,7 @@ describe Geo::AttachmentRegistryFinder, :geo do
         end
 
         it 'counts attachments that has been synced' do
-          create(:geo_file_registry, :attachment, file_id: upload_1.id, success: false)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
           create(:geo_file_registry, :attachment, file_id: upload_2.id)
           create(:geo_file_registry, :attachment, file_id: upload_3.id)
 
@@ -177,17 +177,17 @@ describe Geo::AttachmentRegistryFinder, :geo do
       end
 
       it 'counts attachments that sync has failed' do
-        create(:geo_file_registry, :attachment, file_id: upload_1.id, success: false)
+        create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
         create(:geo_file_registry, :attachment, file_id: upload_2.id)
-        create(:geo_file_registry, :attachment, file_id: upload_3.id, success: false)
+        create(:geo_file_registry, :attachment, :failed, file_id: upload_3.id)
 
         expect(subject.count_failed).to eq 2
       end
 
       it 'ignores remote attachments' do
-        create(:geo_file_registry, :attachment, file_id: upload_remote_1.id, success: false)
-        create(:geo_file_registry, :attachment, file_id: upload_2.id, success: false)
-        create(:geo_file_registry, :attachment, file_id: upload_3.id, success: false)
+        create(:geo_file_registry, :attachment, :failed, file_id: upload_remote_1.id)
+        create(:geo_file_registry, :attachment, :failed, file_id: upload_2.id)
+        create(:geo_file_registry, :attachment, :failed, file_id: upload_3.id)
 
         expect(subject.count_failed).to eq 2
       end
@@ -204,22 +204,22 @@ describe Geo::AttachmentRegistryFinder, :geo do
         end
 
         it 'counts attachments that sync has failed' do
-          create(:geo_file_registry, :attachment, file_id: upload_1.id, success: false)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
           create(:geo_file_registry, :attachment, file_id: upload_3.id)
 
           expect(subject.count_failed).to eq 1
         end
 
         it 'does not count attachments of unsynced projects' do
-          create(:geo_file_registry, :attachment, file_id: upload_2.id, success: false)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_2.id)
 
           expect(subject.count_failed).to eq 0
         end
 
         it 'ignores remote attachments' do
-          create(:geo_file_registry, :attachment, file_id: upload_1.id, success: false)
-          create(:geo_file_registry, :attachment, file_id: upload_2.id, success: false)
-          create(:geo_file_registry, :attachment, file_id: upload_3.id, success: false)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_2.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_3.id)
           upload_1.update!(store: ObjectStorage::Store::REMOTE)
 
           expect(subject.count_failed).to eq 1
@@ -238,22 +238,22 @@ describe Geo::AttachmentRegistryFinder, :geo do
         end
 
         it 'counts attachments that sync has failed' do
-          create(:geo_file_registry, :attachment, file_id: upload_1.id, success: false)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
           create(:geo_file_registry, :attachment, file_id: upload_3.id)
 
           expect(subject.count_failed).to eq 1
         end
 
         it 'does not count attachments of unsynced projects' do
-          create(:geo_file_registry, :attachment, file_id: upload_2.id, success: false)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_2.id)
 
           expect(subject.count_failed).to eq 0
         end
 
         it 'ignores remote attachments' do
-          create(:geo_file_registry, :attachment, file_id: upload_1.id, success: false)
-          create(:geo_file_registry, :attachment, file_id: upload_2.id, success: false)
-          create(:geo_file_registry, :attachment, file_id: upload_3.id, success: false)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_2.id)
+          create(:geo_file_registry, :attachment, :failed, file_id: upload_3.id)
           upload_1.update!(store: ObjectStorage::Store::REMOTE)
 
           expect(subject.count_failed).to eq 1
@@ -292,7 +292,7 @@ describe Geo::AttachmentRegistryFinder, :geo do
       end
 
       it 'excludes attachments that are not synced' do
-        create(:geo_file_registry, :attachment, file_id: upload_1.id, success: false, missing_on_primary: true)
+        create(:geo_file_registry, :attachment, :failed, file_id: upload_1.id, missing_on_primary: true)
 
         expect(subject.count_synced_missing_on_primary).to eq 0
       end
@@ -340,7 +340,7 @@ describe Geo::AttachmentRegistryFinder, :geo do
       end
 
       it 'returns uploads without an entry on the tracking database' do
-        create(:geo_file_registry, :avatar, file_id: upload_1.id, success: true)
+        create(:geo_file_registry, :avatar, file_id: upload_1.id)
 
         uploads = subject.find_unsynced(batch_size: 10)
 
@@ -397,8 +397,8 @@ describe Geo::AttachmentRegistryFinder, :geo do
       it 'excludes except_file_ids' do
         upload_a = create(:upload, :object_storage, model: synced_group)
         upload_b = create(:upload, :object_storage, model: unsynced_group)
-        create(:geo_file_registry, :avatar, file_id: upload_a.id, success: true)
-        create(:geo_file_registry, :avatar, file_id: upload_b.id, success: true)
+        create(:geo_file_registry, :avatar, file_id: upload_a.id)
+        create(:geo_file_registry, :avatar, file_id: upload_b.id)
 
         uploads = subject.find_migrated_local(batch_size: 10, except_file_ids: [upload_a.id])
 

ee/spec/finders/geo/lfs_object_registry_finder_spec.rb

@@ -87,7 +87,7 @@ describe Geo::LfsObjectRegistryFinder, :geo do
       end
 
       it 'counts LFS objects that has been synced' do
-        create(:geo_file_registry, :lfs, file_id: lfs_object_1.id, success: false)
+        create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_1.id)
         create(:geo_file_registry, :lfs, file_id: lfs_object_2.id)
         create(:geo_file_registry, :lfs, file_id: lfs_object_3.id)
 
@@ -120,7 +120,7 @@ describe Geo::LfsObjectRegistryFinder, :geo do
         end
 
         it 'counts LFS objects that has been synced' do
-          create(:geo_file_registry, :lfs, file_id: lfs_object_1.id, success: false)
+          create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_1.id)
           create(:geo_file_registry, :lfs, file_id: lfs_object_2.id)
           create(:geo_file_registry, :lfs, file_id: lfs_object_3.id)
 
@@ -156,17 +156,17 @@ describe Geo::LfsObjectRegistryFinder, :geo do
       end
 
       it 'counts LFS objects that sync has failed' do
-        create(:geo_file_registry, :lfs, file_id: lfs_object_1.id, success: false)
+        create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_1.id)
         create(:geo_file_registry, :lfs, file_id: lfs_object_2.id)
-        create(:geo_file_registry, :lfs, file_id: lfs_object_3.id, success: false)
+        create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_3.id)
 
         expect(subject.count_failed).to eq 2
       end
 
       it 'ignores remote LFS objects' do
-        create(:geo_file_registry, :lfs, file_id: lfs_object_remote_1.id, success: false)
-        create(:geo_file_registry, :lfs, file_id: lfs_object_2.id, success: false)
-        create(:geo_file_registry, :lfs, file_id: lfs_object_3.id, success: false)
+        create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_remote_1.id)
+        create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_2.id)
+        create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_3.id)
 
         expect(subject.count_failed).to eq 2
       end
@@ -189,17 +189,17 @@ describe Geo::LfsObjectRegistryFinder, :geo do
         end
 
         it 'counts LFS objects that sync has failed' do
-          create(:geo_file_registry, :lfs, file_id: lfs_object_1.id, success: false)
+          create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_1.id)
           create(:geo_file_registry, :lfs, file_id: lfs_object_2.id)
-          create(:geo_file_registry, :lfs, file_id: lfs_object_3.id, success: false)
+          create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_3.id)
 
           expect(subject.count_failed).to eq 1
         end
 
         it 'ignores remote LFS objects' do
-          create(:geo_file_registry, :lfs, file_id: lfs_object_1.id, success: false)
-          create(:geo_file_registry, :lfs, file_id: lfs_object_2.id, success: false)
-          create(:geo_file_registry, :lfs, file_id: lfs_object_3.id, success: false)
+          create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_1.id)
+          create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_2.id)
+          create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_3.id)
           lfs_object_1.update_column(:file_store, ObjectStorage::Store::REMOTE)
 
           expect(subject.count_failed).to eq 1
@@ -238,7 +238,7 @@ describe Geo::LfsObjectRegistryFinder, :geo do
       end
 
       it 'excludes LFS objects that are not synced' do
-        create(:geo_file_registry, :lfs, file_id: lfs_object_1.id, success: false, missing_on_primary: true)
+        create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_1.id, missing_on_primary: true)
 
         expect(subject.count_synced_missing_on_primary).to eq 0
       end
@@ -292,8 +292,8 @@ describe Geo::LfsObjectRegistryFinder, :geo do
       end
 
       it 'returns LFS objects without an entry on the tracking database' do
-        create(:geo_file_registry, :lfs, file_id: lfs_object_1.id, success: true)
-        create(:geo_file_registry, :lfs, file_id: lfs_object_3.id, success: false)
+        create(:geo_file_registry, :lfs, file_id: lfs_object_1.id)
+        create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_3.id)
 
         lfs_objects = subject.find_unsynced(batch_size: 10)
 
@@ -301,8 +301,8 @@ describe Geo::LfsObjectRegistryFinder, :geo do
       end
 
       it 'excludes LFS objects without an entry on the tracking database' do
-        create(:geo_file_registry, :lfs, file_id: lfs_object_1.id, success: true)
-        create(:geo_file_registry, :lfs, file_id: lfs_object_3.id, success: false)
+        create(:geo_file_registry, :lfs, file_id: lfs_object_1.id)
+        create(:geo_file_registry, :lfs, :failed, file_id: lfs_object_3.id)
 
         lfs_objects = subject.find_unsynced(batch_size: 10, except_file_ids: [lfs_object_2.id])
 

ee/spec/models/geo_node_status_spec.rb

@@ -131,7 +131,7 @@ describe GeoNodeStatus, :geo, :delete do
 
       create(:geo_file_registry, :avatar, file_id: uploads[0])
       create(:geo_file_registry, :avatar, file_id: uploads[1])
-      create(:geo_file_registry, :avatar, file_id: uploads[2], success: false)
+      create(:geo_file_registry, :avatar, :failed, file_id: uploads[2])
 
       expect(subject.attachments_synced_count).to eq(2)
     end
@@ -176,7 +176,7 @@ describe GeoNodeStatus, :geo, :delete do
 
       create(:geo_file_registry, :avatar, file_id: uploads[0], missing_on_primary: true)
       create(:geo_file_registry, :avatar, file_id: uploads[1])
-      create(:geo_file_registry, :avatar, file_id: uploads[2], success: false)
+      create(:geo_file_registry, :avatar, :failed, file_id: uploads[2])
 
       expect(subject.attachments_synced_missing_on_primary_count).to eq(1)
     end
@@ -185,13 +185,13 @@ describe GeoNodeStatus, :geo, :delete do
   describe '#attachments_failed_count', :delete do
     it 'counts failed avatars, attachment, personal snippets and files' do
       # These two should be ignored
-      create(:geo_file_registry, :lfs, :with_file, success: false)
+      create(:geo_file_registry, :lfs, :with_file, :failed)
       create(:geo_file_registry, :with_file)
 
-      create(:geo_file_registry, :with_file, file_type: :personal_file, success: false)
-      create(:geo_file_registry, :with_file, file_type: :attachment, success: false)
-      create(:geo_file_registry, :avatar, :with_file, success: false)
-      create(:geo_file_registry, :with_file, success: false)
+      create(:geo_file_registry, :with_file, :failed, file_type: :personal_file)
+      create(:geo_file_registry, :with_file, :failed, file_type: :attachment)
+      create(:geo_file_registry, :avatar, :with_file, :failed)
+      create(:geo_file_registry, :with_file, :failed)
 
       expect(subject.attachments_failed_count).to eq(4)
     end
@@ -248,12 +248,12 @@ describe GeoNodeStatus, :geo, :delete do
   describe '#lfs_objects_synced_count', :delete do
     it 'counts synced LFS objects' do
       # These four should be ignored
-      create(:geo_file_registry, success: false)
+      create(:geo_file_registry, :failed)
       create(:geo_file_registry, :avatar)
       create(:geo_file_registry, file_type: :attachment)
-      create(:geo_file_registry, :lfs, :with_file, success: false)
+      create(:geo_file_registry, :lfs, :with_file, :failed)
 
-      create(:geo_file_registry, :lfs, :with_file, success: true)
+      create(:geo_file_registry, :lfs, :with_file)
 
       expect(subject.lfs_objects_synced_count).to eq(1)
     end
@@ -264,12 +264,12 @@ describe GeoNodeStatus, :geo, :delete do
   describe '#lfs_objects_synced_missing_on_primary_count', :delete do
     it 'counts LFS objects marked as synced due to file missing on the primary' do
       # These four should be ignored
-      create(:geo_file_registry, success: false)
+      create(:geo_file_registry, :failed)
       create(:geo_file_registry, :avatar, missing_on_primary: true)
       create(:geo_file_registry, file_type: :attachment, missing_on_primary: true)
-      create(:geo_file_registry, :lfs, :with_file, success: false)
+      create(:geo_file_registry, :lfs, :with_file, :failed)
 
-      create(:geo_file_registry, :lfs, :with_file, success: true, missing_on_primary: true)
+      create(:geo_file_registry, :lfs, :with_file, missing_on_primary: true)
 
       expect(subject.lfs_objects_synced_missing_on_primary_count).to eq(1)
     end
@@ -280,12 +280,12 @@ describe GeoNodeStatus, :geo, :delete do
   describe '#lfs_objects_failed_count', :delete do
     it 'counts failed LFS objects' do
       # These four should be ignored
-      create(:geo_file_registry, success: false)
-      create(:geo_file_registry, :avatar, success: false)
-      create(:geo_file_registry, file_type: :attachment, success: false)
+      create(:geo_file_registry, :failed)
+      create(:geo_file_registry, :avatar, :failed)
+      create(:geo_file_registry, :failed, file_type: :attachment)
       create(:geo_file_registry, :lfs, :with_file)
 
-      create(:geo_file_registry, :lfs, :with_file, success: false)
+      create(:geo_file_registry, :lfs, :with_file, :failed)
 
       expect(subject.lfs_objects_failed_count).to eq(1)
     end
@@ -308,14 +308,14 @@ describe GeoNodeStatus, :geo, :delete do
     end
 
     it 'returns the right percentage with no group restrictions' do
-      create(:geo_file_registry, :lfs, file_id: lfs_object_project.lfs_object_id, success: true)
+      create(:geo_file_registry, :lfs, file_id: lfs_object_project.lfs_object_id)
 
       expect(subject.lfs_objects_synced_in_percentage).to be_within(0.0001).of(25)
     end
 
     it 'returns the right percentage with group restrictions' do
       secondary.update!(selective_sync_type: 'namespaces', namespaces: [group])
-      create(:geo_file_registry, :lfs, file_id: lfs_object_project.lfs_object_id, success: true)
+      create(:geo_file_registry, :lfs, file_id: lfs_object_project.lfs_object_id)
 
       expect(subject.lfs_objects_synced_in_percentage).to be_within(0.0001).of(50)
     end
@@ -326,7 +326,7 @@ describe GeoNodeStatus, :geo, :delete do
   describe '#job_artifacts_synced_count', :delete do
     it 'counts synced job artifacts' do
       # These should be ignored
-      create(:geo_file_registry, success: true)
+      create(:geo_file_registry)
       create(:geo_job_artifact_registry, :with_artifact, success: false)
 
       create(:geo_job_artifact_registry, :with_artifact, success: true)
@@ -340,7 +340,7 @@ describe GeoNodeStatus, :geo, :delete do
   describe '#job_artifacts_synced_missing_on_primary_count', :delete do
     it 'counts job artifacts marked as synced due to file missing on the primary' do
       # These should be ignored
-      create(:geo_file_registry, success: true, missing_on_primary: true)
+      create(:geo_file_registry, missing_on_primary: true)
       create(:geo_job_artifact_registry, :with_artifact, success: true)
 
       create(:geo_job_artifact_registry, :with_artifact, success: true, missing_on_primary: true)
@@ -354,9 +354,9 @@ describe GeoNodeStatus, :geo, :delete do
   describe '#job_artifacts_failed_count', :delete do
     it 'counts failed job artifacts' do
       # These should be ignored
-      create(:geo_file_registry, success: false)
-      create(:geo_file_registry, :avatar, success: false)
-      create(:geo_file_registry, file_type: :attachment, success: false)
+      create(:geo_file_registry, :failed)
+      create(:geo_file_registry, :avatar, :failed)
+      create(:geo_file_registry, :attachment, :failed)
       create(:geo_job_artifact_registry, :with_artifact, success: true)
 
       create(:geo_job_artifact_registry, :with_artifact, success: false)

ee/spec/workers/geo/file_download_dispatch_worker_spec.rb

@@ -56,7 +56,7 @@ describe Geo::FileDownloadDispatchWorker, :geo do
       end
 
       it 'performs Geo::FileDownloadWorker for failed-sync attachments' do
-        create(:geo_file_registry, :avatar, file_id: upload.id, bytes: 0, success: false)
+        create(:geo_file_registry, :avatar, :failed, file_id: upload.id, bytes: 0)
 
         expect(Geo::FileDownloadWorker).to receive(:perform_async)
           .with('avatar', upload.id).once.and_return(spy)
@@ -65,7 +65,7 @@ describe Geo::FileDownloadDispatchWorker, :geo do
       end
 
       it 'does not perform Geo::FileDownloadWorker for synced attachments' do
-        create(:geo_file_registry, :avatar, file_id: upload.id, bytes: 1234, success: true)
+        create(:geo_file_registry, :avatar, file_id: upload.id, bytes: 1234)
 
         expect(Geo::FileDownloadWorker).not_to receive(:perform_async)
 
@@ -73,7 +73,7 @@ describe Geo::FileDownloadDispatchWorker, :geo do
       end
 
       it 'does not perform Geo::FileDownloadWorker for synced attachments even with 0 bytes downloaded' do
-        create(:geo_file_registry, :avatar, file_id: upload.id, bytes: 0, success: true)
+        create(:geo_file_registry, :avatar, file_id: upload.id, bytes: 0)
 
         expect(Geo::FileDownloadWorker).not_to receive(:perform_async)
 
@@ -81,7 +81,7 @@ describe Geo::FileDownloadDispatchWorker, :geo do
       end
 
       context 'with a failed file' do
-        let(:failed_registry) { create(:geo_file_registry, :avatar, file_id: 999, success: false) }
+        let(:failed_registry) { create(:geo_file_registry, :avatar, :failed, file_id: 999) }
 
         it 'does not stall backfill' do
           unsynced = create(:upload)
@@ -101,7 +101,7 @@ describe Geo::FileDownloadDispatchWorker, :geo do
         end
 
         it 'does not retry failed files when retry_at is tomorrow' do
-          failed_registry = create(:geo_file_registry, :avatar, file_id: 999, success: false, retry_at: Date.tomorrow)
+          failed_registry = create(:geo_file_registry, :avatar, :failed, file_id: 999, retry_at: Date.tomorrow)
 
           expect(Geo::FileDownloadWorker).not_to receive(:perform_async).with('avatar', failed_registry.file_id)
 
@@ -109,7 +109,7 @@ describe Geo::FileDownloadDispatchWorker, :geo do
         end
 
         it 'retries failed files when retry_at is in the past' do
-          failed_registry = create(:geo_file_registry, :avatar, file_id: 999, success: false, retry_at: Date.yesterday)
+          failed_registry = create(:geo_file_registry, :avatar, :failed, file_id: 999, retry_at: Date.yesterday)
 
           expect(Geo::FileDownloadWorker).to receive(:perform_async).with('avatar', failed_registry.file_id)