diff --git a/src/libslic3r/Preset.cpp b/src/libslic3r/Preset.cpp index 2a64bed1db..f4e6de9b76 100644 --- a/src/libslic3r/Preset.cpp +++ b/src/libslic3r/Preset.cpp @@ -2786,7 +2786,7 @@ std::vector PresetCollection::legacy_user_filament_pre // files of presets whose parent has an entry under one of those are read. if (!has_entry(parent->name) && std::none_of(parent->renamed_from.begin(), parent->renamed_from.end(), has_entry)) continue; - LegacyUserFilamentPreset legacy{preset.name, parent->name, {}, {}}; + LegacyUserFilamentPreset legacy{preset.name, parent->name, {}, {}, preset.file, own}; try { const json j = read_user_preset_file(preset.file); // A preset whose file sets compatible_printers, even to an empty list, was given its printers on @@ -2802,8 +2802,10 @@ std::vector PresetCollection::legacy_user_filament_pre if (it == printers_by_parent.end()) continue; for (const std::string &printer : it->second) - if (std::find(own.begin(), own.end(), printer) == own.end()) + if (std::find(own.begin(), own.end(), printer) == own.end()) { legacy.printers.push_back(printer); + legacy.compatible_printers.push_back(printer); + } if (!legacy.printers.empty()) legacy_presets.push_back(std::move(legacy)); } @@ -2811,61 +2813,47 @@ std::vector PresetCollection::legacy_user_filament_pre return legacy_presets; } -bool PresetCollection::migrate_legacy_user_filament_preset(const LegacyUserFilamentPreset &legacy) +bool PresetCollection::write_legacy_user_filament_preset(const LegacyUserFilamentPreset &legacy) { - // Copied out under the lock: the preset sync thread can delete presets, which invalidates references into - // m_presets, while the file is written. - lock(); - auto it = this->find_preset_internal(legacy.preset); - if (it == m_presets.end() || it->name != legacy.preset || !it->is_user()) { - unlock(); - return false; - } - const std::string file = it->file; - std::vector printers = it->config.option("compatible_printers")->values; - unlock(); - for (const std::string &printer : legacy.printers) - if (std::find(printers.begin(), printers.end(), printer) == printers.end()) - printers.push_back(printer); - // Only compatible_printers changes in the file. Saving the whole preset would also rewrite an inherits that // names a renamed parent by its old name, the only name older releases know it by. - { - const std::string lock_path = user_presets_lock_path(); - InstanceLock instance_lock(lock_path); - // Without the lock another instance could save between the read and the write. Migrating can wait. - if (!lock_path.empty() && !instance_lock.locked()) + try { + json j = read_user_preset_file(legacy.file); + // Changed by hand or by another instance since the preset was found: a list of its own stands, and + // another parent would need other printers. + if (j.contains("compatible_printers") || j.value(BBL_JSON_KEY_INHERITS, std::string()) != legacy.inherits) return false; - try { - json j = read_user_preset_file(file); - // Changed by hand or by another instance since the preset was found: a list of its own stands, and - // another parent would need other printers. - if (j.contains("compatible_printers") || j.value(BBL_JSON_KEY_INHERITS, std::string()) != legacy.inherits) - return false; - j["compatible_printers"] = printers; - if (const std::error_code ec = write_file_atomically(file, j.dump(1, '\t') + "\n")) { - BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": failed to save " << file << ": " << ec.message(); - return false; - } - } catch (const std::exception &err) { - BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": can not migrate " << file << ": " << err.what(); + j["compatible_printers"] = legacy.compatible_printers; + if (const std::error_code ec = write_file_atomically(legacy.file, j.dump(1, '\t') + "\n")) { + BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": failed to save " << legacy.file << ": " << ec.message(); return false; } + } catch (const std::exception &err) { + BOOST_LOG_TRIVIAL(error) << __FUNCTION__ << ": can not migrate " << legacy.file << ": " << err.what(); + return false; } + BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ": user preset " << legacy.preset << " gets " << legacy.printers.size() + << " printer(s) its parent listed in an older release"; + return true; +} +void PresetCollection::apply_legacy_user_filament_presets(const std::vector &written) +{ lock(); - it = this->find_preset_internal(legacy.preset); - if (it != m_presets.end() && it->name == legacy.preset) { - std::vector &saved = it->config.option("compatible_printers", true)->values; + for (const LegacyUserFilamentPreset &legacy : written) { + auto it = this->find_preset_internal(legacy.preset); + if (it == m_presets.end() || it->name != legacy.preset) + continue; // deleted by the preset sync thread meanwhile + std::vector &saved = it->config.option("compatible_printers", true)->values; std::vector &edited = m_edited_preset.config.option("compatible_printers", true)->values; // A change to the list pending in the settings tab stays pending. Without one, the edited copy and the // snapshot unsaved changes are measured against take the written list, so nothing reads as modified. if (m_idx_selected == static_cast(it - m_presets.begin()) && edited == saved) { - edited = printers; + edited = legacy.compatible_printers; if (m_saved_preset.name == it->name) - m_saved_preset.config.option("compatible_printers", true)->values = printers; + m_saved_preset.config.option("compatible_printers", true)->values = legacy.compatible_printers; } - saved = printers; + saved = legacy.compatible_printers; // Push the change to the cloud copy, as an edit in the settings tab would. if (!it->setting_id.empty()) { it->sync_info = "update"; @@ -2873,9 +2861,6 @@ bool PresetCollection::migrate_legacy_user_filament_preset(const LegacyUserFilam } } unlock(); - BOOST_LOG_TRIVIAL(info) << __FUNCTION__ << ": user preset " << legacy.preset << " gets " << legacy.printers.size() - << " printer(s) its parent listed in an older release"; - return true; } //BBS: validate_preset diff --git a/src/libslic3r/Preset.hpp b/src/libslic3r/Preset.hpp index cdbb5fc7d1..98e7e62e90 100644 --- a/src/libslic3r/Preset.hpp +++ b/src/libslic3r/Preset.hpp @@ -558,7 +558,11 @@ struct LegacyUserFilamentPreset { std::string parent; // The parent's name as the preset file states it: the name the older release knew it by. std::string inherits; + // The printers the preset lost: what the prompt lists. std::vector printers; + std::string file; + // The list the file gets: the parent's current printers plus the lost ones. + std::vector compatible_printers; }; // Collections of presets of the same type (one of the Print, Filament or Printer type). @@ -617,10 +621,12 @@ public: // no longer lists. printers_by_parent is keyed by the parent's name in that release, which is the name // the preset file's inherits still holds. std::vector legacy_user_filament_presets(const std::map> &printers_by_parent); - // Adds the printers to the preset's own compatible_printers and saves the preset to disk. False if the - // preset file sets compatible_printers or names another parent by now, if another instance holds the - // user presets lock, or if the file could not be saved. - bool migrate_legacy_user_filament_preset(const LegacyUserFilamentPreset &legacy); + // Writes legacy.compatible_printers into the preset's file and keeps every other key. The caller holds the + // user presets lock. False if the file has gained a compatible_printers key or another parent since the + // preset was found, or could not be written. + static bool write_legacy_user_filament_preset(const LegacyUserFilamentPreset &legacy); + // Gives the loaded presets the lists their files were just given, and marks cloud copies for upload. + void apply_legacy_user_filament_presets(const std::vector &written); //BBS: get user presets int get_user_presets(PresetBundle *preset_bundle, std::vector &result_presets); void set_sync_info_and_save(std::string name, std::string setting_id, std::string syncinfo, long long update_time, const std::string& user_id); diff --git a/src/libslic3r/PresetBundle.cpp b/src/libslic3r/PresetBundle.cpp index 13a02c80b7..0b00eb35d4 100644 --- a/src/libslic3r/PresetBundle.cpp +++ b/src/libslic3r/PresetBundle.cpp @@ -1101,16 +1101,32 @@ std::vector PresetBundle::legacy_user_filament_presets return filaments.legacy_user_filament_presets(legacy_compatible_printers()); } -std::string PresetBundle::migrate_legacy_user_filament_presets(const std::vector &legacy_presets, std::vector &failed) +PresetBundle::LegacyUserFilamentPresetMigration PresetBundle::migrate_legacy_user_filament_presets(const std::vector &legacy_presets) { - const std::string zip_path = data_dir() + "/filament_preset_migration_" + - Utils::time2str(Utils::TimeZone::utc, Utils::TimeFormat::iso8601Z) + "_backup.zip"; - if (!backup_user_folder_to_zip(zip_path)) - return {}; - for (const LegacyUserFilamentPreset &legacy : legacy_presets) - if (!filaments.migrate_legacy_user_filament_preset(legacy)) - failed.push_back(legacy.preset); - return zip_path; + LegacyUserFilamentPresetMigration result; + std::vector written; + { + // One hold for the backup and every write: no other instance can save a preset in between, and a lock + // the other instance keeps is one outcome rather than a failure per preset (the lock backs off for a + // while after a miss). Held without the collection mutex, the order InstanceLock requires. + const std::string lock_path = user_presets_lock_path(); + InstanceLock instance_lock(lock_path); + if (!lock_path.empty() && !instance_lock.locked()) + return result; + result.locked = true; + const std::string zip_path = data_dir() + "/filament_preset_migration_" + + Utils::time2str(Utils::TimeZone::utc, Utils::TimeFormat::iso8601Z) + "_backup.zip"; + if (!backup_user_folder_to_zip(zip_path)) + return result; + result.backup = zip_path; + for (const LegacyUserFilamentPreset &legacy : legacy_presets) + if (PresetCollection::write_legacy_user_filament_preset(legacy)) + written.push_back(legacy); + else + result.failed.push_back(legacy.preset); + } + filaments.apply_legacy_user_filament_presets(written); + return result; } std::optional PresetBundle::get_filament_by_filament_id(const std::string& filament_id, const std::string& printer_name) const diff --git a/src/libslic3r/PresetBundle.hpp b/src/libslic3r/PresetBundle.hpp index 53d96fcf6d..4a3dd057c2 100644 --- a/src/libslic3r/PresetBundle.hpp +++ b/src/libslic3r/PresetBundle.hpp @@ -382,10 +382,15 @@ public: // User filament presets made against an older release that no longer show on printers their parent // listed then (resources/info/legacy_compatible_printers.json). std::vector legacy_user_filament_presets(); - // Backs up the user preset folder into a zip in data_dir(), then writes each preset's printers into its - // file. Returns the zip's path, or an empty string when the backup failed and nothing was migrated. - // The presets that could not be migrated are added to failed. - std::string migrate_legacy_user_filament_presets(const std::vector &legacy_presets, std::vector &failed); + // Under one hold of the user presets lock, backs up the user preset folder into a zip in data_dir() and writes + // each preset's printers into its file. Nothing is written when the lock is held elsewhere (locked false) or + // the backup failed (backup empty); otherwise failed names the presets whose files could not be written. + struct LegacyUserFilamentPresetMigration { + bool locked{false}; + std::string backup; + std::vector failed; + }; + LegacyUserFilamentPresetMigration migrate_legacy_user_filament_presets(const std::vector &legacy_presets); //BBS: project embedded preset logic PresetsConfigSubstitutions load_project_embedded_presets(std::vector project_presets, ForwardCompatibilitySubstitutionRule substitution_rule); diff --git a/src/slic3r/GUI/GUI_App.cpp b/src/slic3r/GUI/GUI_App.cpp index d3eb49aeaf..a1f9abfe0a 100644 --- a/src/slic3r/GUI/GUI_App.cpp +++ b/src/slic3r/GUI/GUI_App.cpp @@ -1109,10 +1109,7 @@ void GUI_App::post_init() } this->check_new_version_sf(); - // Only presets made against 2.4.2 or older, the release legacy_compatible_printers.json is generated - // against, can be missing printers, so the prompt runs only on the first start after one of those. - if (!cw_showed && m_last_config_version && m_last_config_version->valid() - && *m_last_config_version <= Semver(2, 4, 2)) + if (!cw_showed) this->check_legacy_user_filament_presets(false); const auto cloud_provider = get_printer_cloud_provider(); if (is_user_login(cloud_provider) && !app_config->get_stealth_mode()) { @@ -6852,8 +6849,14 @@ void GUI_App::check_legacy_user_filament_presets(bool by_user) if (!migrate) return; - std::vector failed; - const std::string backup = preset_bundle->migrate_legacy_user_filament_presets(legacy_presets, failed); + const PresetBundle::LegacyUserFilamentPresetMigration migration = preset_bundle->migrate_legacy_user_filament_presets(legacy_presets); + if (!migration.locked) { + show_error(mainframe, _L("Another OrcaSlicer instance is saving presets right now, so nothing was migrated. " + "Try again later from Help > Migrate Filament Presets.")); + return; + } + const std::string &backup = migration.backup; + const std::vector &failed = migration.failed; if (backup.empty()) { show_error(mainframe, _L("Your presets could not be backed up, so they were not migrated.")); return; diff --git a/src/slic3r/GUI/GUI_App.hpp b/src/slic3r/GUI/GUI_App.hpp index 7ab9dd58a9..0dcdc8e36e 100644 --- a/src/slic3r/GUI/GUI_App.hpp +++ b/src/slic3r/GUI/GUI_App.hpp @@ -568,8 +568,8 @@ public: void check_new_version(bool show_tips = false, int by_user = 0); void check_new_version_sf(bool show_tips = false, int by_user = 0); // Offers to migrate user filament presets made against an older release that lost printers their parent - // listed then. At startup it runs only on the first start after 2.4.2 or older, unless the user chose not - // to be reminded; by_user also reports when there is nothing to migrate. + // listed then. At startup it asks on every start that finds such presets, unless the user chose not to be + // reminded; by_user also reports when there is nothing to migrate. void check_legacy_user_filament_presets(bool by_user); bool process_network_msg(std::string dev_id, std::string msg); void request_new_version(int by_user); diff --git a/tests/libslic3r/test_preset_bundle_loading.cpp b/tests/libslic3r/test_preset_bundle_loading.cpp index 64fde37396..edba12c2b1 100644 --- a/tests/libslic3r/test_preset_bundle_loading.cpp +++ b/tests/libslic3r/test_preset_bundle_loading.cpp @@ -7329,6 +7329,8 @@ TEST_CASE("User filaments made against an older release that lost printers their CHECK(legacy_presets[0].parent == k1_parent); CHECK(legacy_presets[0].inherits == k1_old_parent); CHECK(legacy_presets[0].printers == std::vector{ "Creality K1C 0.4 nozzle" }); + CHECK(legacy_presets[0].file == (temp_dir.path() / PRESET_FILAMENT_NAME / "Old PETG.json").string()); + CHECK(legacy_presets[0].compatible_printers == std::vector{ "Creality K1C 0.6 nozzle", "Creality K1C 0.4 nozzle" }); } TEST_CASE("Migrating a user filament backs up the user folder to a zip and writes the legacy printers to its file", "[Preset][Bundle]") @@ -7345,11 +7347,12 @@ TEST_CASE("Migrating a user filament backs up the user folder to a zip and write const std::vector legacy_presets = bundle.filaments.legacy_user_filament_presets(map); REQUIRE(legacy_presets.size() == 1); - std::vector failed; - const std::string backup = bundle.migrate_legacy_user_filament_presets(legacy_presets, failed); + const PresetBundle::LegacyUserFilamentPresetMigration migration = bundle.migrate_legacy_user_filament_presets(legacy_presets); + const std::string &backup = migration.backup; + CHECK(migration.locked); REQUIRE_FALSE(backup.empty()); CHECK(std::regex_match(fs::path(backup).filename().string(), std::regex("filament_preset_migration_\\d{8}T\\d{6}Z_backup\\.zip"))); - CHECK(failed.empty()); + CHECK(migration.failed.empty()); const fs::path extracted = temp_dir.path() / "extracted"; REQUIRE(extract_archive_confined(backup, extracted.string())); CHECK(read_file(extracted / PRESET_USER_DIR / "default" / PRESET_FILAMENT_NAME / "My PETG.json") == original); @@ -7361,7 +7364,8 @@ TEST_CASE("Migrating a user filament backs up the user folder to a zip and write const std::vector expected{ "Creality K1C 0.6 nozzle", "Creality K1C 0.4 nozzle" }; CHECK(reloaded.filaments.find_preset("My PETG")->config.option("compatible_printers")->values == expected); CHECK(reloaded.filaments.legacy_user_filament_presets(map).empty()); - CHECK_FALSE(reloaded.filaments.migrate_legacy_user_filament_preset(legacy_presets[0])); + // The file now sets its own list, so the migration found earlier no longer applies to it. + CHECK(reloaded.migrate_legacy_user_filament_presets(legacy_presets).failed == std::vector{ "My PETG" }); } TEST_CASE("Migrating a user filament keeps the old name of its renamed parent", "[Preset][Bundle]") @@ -7376,7 +7380,7 @@ TEST_CASE("Migrating a user filament keeps the old name of its renamed parent", const std::vector legacy_presets = filaments.legacy_user_filament_presets({ { k1_old_parent, { "Creality K1C 0.4 nozzle" } } }); REQUIRE(legacy_presets.size() == 1); - REQUIRE(filaments.migrate_legacy_user_filament_preset(legacy_presets[0])); + REQUIRE(PresetCollection::write_legacy_user_filament_preset(legacy_presets[0])); // An older release knows the parent only by its old name. const std::string saved = read_file(file); CHECK(saved.find("\"inherits\": \"" + k1_old_parent + "\"") != std::string::npos); @@ -7402,27 +7406,37 @@ TEST_CASE("Migrating a user filament leaves a file another instance changed afte write_user_filament(temp_dir.path(), "My PETG", "Generic PETG @System"); } const std::string edited = read_file(file); - CHECK_FALSE(filaments.migrate_legacy_user_filament_preset(legacy_presets[0])); + CHECK_FALSE(PresetCollection::write_legacy_user_filament_preset(legacy_presets[0])); CHECK(read_file(file) == edited); } -TEST_CASE("Migrating a user filament writes nothing while it can not take the user presets lock", "[Preset][Bundle]") +TEST_CASE("Migrating user filaments backs up and writes nothing while it can not take the user presets lock", "[Preset][Bundle]") { - ScopedTemporaryDir temp_dir; - RenameTestFilaments filaments; - add_system_filament(filaments, k1_parent, { "Creality K1C 0.6 nozzle" }); - const fs::path file = write_user_filament(temp_dir.path(), "My PETG", k1_parent); - const std::string original = read_file(file); - load_user_filaments(filaments, temp_dir.path()); + ScopedTemporaryDir temp_dir; + ScopedDataDir data(temp_dir.path()); + const fs::path user_dir = temp_dir.path() / PRESET_USER_DIR / "default"; + const fs::path file = write_user_filament(user_dir, "My PETG", k1_parent); + const std::string original = read_file(file); + PresetBundle bundle; + add_system_filament(bundle.filaments, k1_parent, { "Creality K1C 0.6 nozzle" }); + load_user_filaments(bundle.filaments, user_dir); const std::vector legacy_presets = - filaments.legacy_user_filament_presets({ { k1_parent, { "Creality K1C 0.4 nozzle" } } }); + bundle.filaments.legacy_user_filament_presets({ { k1_parent, { "Creality K1C 0.4 nozzle" } } }); REQUIRE(legacy_presets.size() == 1); - // A folder in the lock file's place can not be opened as the lock. - ScopedDataDir data(temp_dir.path() / "data"); + // A folder in the lock file's place can not be opened as the lock; a fresh data dir, since loading has + // already opened the lock file of the first one. One hold covers the backup and every write, so a lock + // held elsewhere is one outcome, not a failure per preset. + const fs::path other_data = temp_dir.path() / "data"; + ScopedDataDir data_for_migration(other_data); fs::create_directories(fs::path(user_presets_lock_path())); - CHECK_FALSE(filaments.migrate_legacy_user_filament_preset(legacy_presets[0])); + const PresetBundle::LegacyUserFilamentPresetMigration migration = bundle.migrate_legacy_user_filament_presets(legacy_presets); + CHECK_FALSE(migration.locked); + CHECK(migration.backup.empty()); + CHECK(migration.failed.empty()); CHECK(read_file(file) == original); + for (const fs::directory_entry &entry : fs::directory_iterator(other_data)) + CHECK(entry.path().extension() != ".zip"); } TEST_CASE("Migrating the selected user filament keeps a printer list edit pending in the settings tab", "[Preset][Bundle]") @@ -7449,7 +7463,8 @@ TEST_CASE("Migrating the selected user filament keeps a printer list edit pendin expected_edited = edited_printers(); expected_dirty = true; } - REQUIRE(filaments.migrate_legacy_user_filament_preset(legacy_presets[0])); + REQUIRE(PresetCollection::write_legacy_user_filament_preset(legacy_presets[0])); + filaments.apply_legacy_user_filament_presets(legacy_presets); // find_preset() returns the edited copy of the selected preset unless asked for the real one. CHECK(filaments.find_preset("My PETG", false, true)->config.option("compatible_printers")->values == migrated); CHECK(edited_printers() == expected_edited);