From 87a96a7ee4fe324bf576a8229b85f0214a079d86 Mon Sep 17 00:00:00 2001 From: James Smith Date: Mon, 27 Jun 2022 22:18:04 +0100 Subject: [PATCH 1/2] don't duplicate previously-merged models during rescan --- app/jobs/model_scan_job.rb | 27 +++++++++++++++++++-------- app/models/library.rb | 1 + spec/jobs/model_scan_job_spec.rb | 27 +++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 8 deletions(-) diff --git a/app/jobs/model_scan_job.rb b/app/jobs/model_scan_job.rb index ea080584c..795da3324 100644 --- a/app/jobs/model_scan_job.rb +++ b/app/jobs/model_scan_job.rb @@ -7,22 +7,31 @@ def self.file_pattern "*.{#{lower.zip(upper).flatten.join(",")}}" end + def model_file_paths(library) + library.model_files.reload.map do |x| + File.join(library.path, x.model.path, x.filename) + end + end + def perform(model) # For each file in the model, create a file object model_path = File.join(model.library.path, model.path) + all_file_paths = model_file_paths(model.library) Dir.open(model_path) do |dir| Dir.glob([ File.join(dir.path, ModelScanJob.file_pattern), File.join(dir.path, "files", ModelScanJob.file_pattern) ]).each do |filename| - # Create the file - file = model.model_files.find_or_create_by(filename: filename.gsub(model_path + "/", "")) - # Try to guess if the file is presupported - if !( - File.join(model_path, filename).split(/[[:punct:]]|[[:space:]]/).map(&:downcase) & - ["presupported", "supported", "sup", "wsupports", "withsupports"] - ).empty? - file.update!(presupported: true) + unless all_file_paths.include?(filename) + # Create the file + file = model.model_files.find_or_create_by(filename: filename.gsub(model_path + "/", "")) + # Try to guess if the file is presupported + if !( + File.join(model_path, filename).split(/[[:punct:]]|[[:space:]]/).map(&:downcase) & + ["presupported", "supported", "sup", "wsupports", "withsupports"] + ).empty? + file.update!(presupported: true) + end end end end @@ -30,6 +39,8 @@ def perform(model) model.model_files.select { |f| !File.exist?(File.join(model_path, f.filename)) }.each(&:destroy) + # If this model has no files, self destruct + model.destroy if model.model_files.reload.count == 0 # Set tags and default files model.model_files.reload model.preview_file = model.model_files.first unless model.preview_file diff --git a/app/models/library.rb b/app/models/library.rb index 9fc56413c..ecca44ec8 100644 --- a/app/models/library.rb +++ b/app/models/library.rb @@ -1,5 +1,6 @@ class Library < ApplicationRecord has_many :models, dependent: :destroy + has_many :model_files, through: :models validates :path, presence: true, uniqueness: true, existing_path: true default_scope { order(:path) } diff --git a/spec/jobs/model_scan_job_spec.rb b/spec/jobs/model_scan_job_spec.rb index cba93f0cd..45719be51 100644 --- a/spec/jobs/model_scan_job_spec.rb +++ b/spec/jobs/model_scan_job_spec.rb @@ -43,5 +43,32 @@ expect { ModelScanJob.perform_now(thing) }.to change { thing.model_files.count }.from(2).to(1) expect(thing.model_files.first.filename).to eq "files/part_one.stl" end + + it "does not recreate parts that have been merged into other models" do + # Set up one model nested inside another + model_one = create(:model, path: "model_one", library: library) + ModelScanJob.perform_now(model_one) + nested_model = create(:model, path: "model_one/nested_model", library: library) + ModelScanJob.perform_now(nested_model) + # Check initial conditions + expect(Model.count).to eq 2 + expect(ModelFile.count).to eq 3 + expect(model_one.model_files.count).to eq 2 + expect(nested_model.model_files.count).to eq 1 + # Merge the models + nested_model.merge_into_parent! + # Check that worked + expect(Model.count).to eq 1 + expect(ModelFile.count).to eq 3 + expect(model_one.model_files.count).to eq 3 + # Now recreate the nested model and rescan + nested_model = create(:model, path: "model_one/nested_model", library: library) + ModelScanJob.perform_now(nested_model) + # That should not have changed anything apart from creating an empty model which will be cleaned up later + expect(Model.count).to eq 1 + expect(ModelFile.count).to eq 3 + expect(model_one.model_files.count).to eq 3 + expect(nested_model.model_files.count).to eq 0 + end end end From 1154ef5aff726b2009bf4d551e5998650d060b44 Mon Sep 17 00:00:00 2001 From: James Smith Date: Mon, 27 Jun 2022 23:43:12 +0100 Subject: [PATCH 2/2] don't try to use empty models after deletion --- app/jobs/model_scan_job.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/jobs/model_scan_job.rb b/app/jobs/model_scan_job.rb index 795da3324..af9de133f 100644 --- a/app/jobs/model_scan_job.rb +++ b/app/jobs/model_scan_job.rb @@ -39,11 +39,11 @@ def perform(model) model.model_files.select { |f| !File.exist?(File.join(model_path, f.filename)) }.each(&:destroy) - # If this model has no files, self destruct - model.destroy if model.model_files.reload.count == 0 # Set tags and default files model.model_files.reload model.preview_file = model.model_files.first unless model.preview_file model.autogenerate_tags_from_path! if model.tags.empty? + # If this model has no files, self destruct + model.destroy if model.model_files.reload.count == 0 end end