Core - Fix profile service, storage and device bugs found by the tests (#912)

* Core - Fix default headset layouts and the z-index of devices placed by the auto-arrange fallback

- Headsets with 2 or 4 LEDs get their default layout, the files are named "... LEDs headset.xml"
- Devices placed by the auto-arrange fallback get the z-index of their type

* Core - Fall back to the default value passed to GetSetting when a stored setting can't be read

* Core - Fix storing, cloning and migrating profiles

- New profiles are stored at the current migration version, so they're no longer migrated on every start and can be duplicated right away
- A category created at the top is listed first right away, so its profiles render on top
- CloneProfileConfiguration makes a copy of the stored data, saving a clone stores its profile through the original
- Importing a profile raises ProfileAdded
- A profile configuration that isn't valid JSON is logged and skipped instead of stopping all migrations
- A stored profile without a version is migrated from version 0, like an imported one
- ProfileCategoryRepository.IsUnique returns true when the name is free
- M0001 and M0004 leave a missing node type or storage out instead of writing null
- M0003 also unwraps a list that is an item of another list

src/Artemis.Core.Tests/Services/DeviceServiceTests.cs

@@ -60,21 +60,16 @@ public class DeviceServiceTests : ServiceTestBase
     }
 
     [Theory]
-    [InlineData("Artemis 1 LED headset.xml", true)]
-    [InlineData("Artemis 2 LEDs headset.xml", false)]
-    [InlineData("Artemis 4 LEDs headset.xml", false)]
-    public void AddDeviceProvider_GivesHeadsetsTheDefaultLayoutForTheirNumberOfLeds(string layoutFile, bool getsLayout)
+    [InlineData("Artemis 1 LED headset.xml")]
+    [InlineData("Artemis 2 LEDs headset.xml")]
+    [InlineData("Artemis 4 LEDs headset.xml")]
+    public void AddDeviceProvider_GivesHeadsetsTheDefaultLayoutForTheirNumberOfLeds(string layoutFile)
     {
         AddDeviceProvider(Path.Combine(AppContext.BaseDirectory, "DefaultLayouts", "Artemis", "Headset", layoutFile));
 
-        // Pinned as-is: the default headset layouts are looked up as "Artemis 2 LED headset.xml" and "Artemis 4 LED
-        // headset.xml", but the files are named "... LEDs headset.xml", so headsets with 2 or more LEDs get no default
-        // layout (the device service logs it as an invalid layout)
         ArtemisDevice headset = Devices.Devices.Single();
-        if (getsLayout)
-            headset.Layout!.IsDefaultLayout.ShouldBeTrue();
-        else
-            headset.Layout.ShouldBeNull();
+        headset.Layout!.IsDefaultLayout.ShouldBeTrue();
+        headset.Layout.FilePath.ShouldEndWith(layoutFile);
     }
 
     [Fact]
@@ -294,9 +289,9 @@ public class DeviceServiceTests : ServiceTestBase
         // There's no fan to place the LED strip next to, so it goes 10 to the right of everything else
         (strip.X, strip.Y).ShouldBe((10 + 441 + 10f, 0f));
 
-        // Pinned as-is: only devices placed by one of their type's own rules get the z-index of their type. LED strips
-        // normally get 2, but placed by the fallback (no fan to place it next to) it keeps the 1 every new device starts with
-        strip.ZIndex.ShouldBe(1);
+        // Placed by the fallback, the LED strip still gets the z-index of its type
+        keyboard.ZIndex.ShouldBe(1);
+        strip.ZIndex.ShouldBe(2);
     }
 
     [Fact]

src/Artemis.Core.Tests/Services/ProfileCategoryTests.cs

@@ -65,16 +65,11 @@ public class ProfileCategoryTests : ProfileServiceTestBase
     }
 
     [Fact]
-    public void CreateProfileCategory_AtTheTop_IsListedLastUntilTheCategoriesAreSortedAgain()
+    public void CreateProfileCategory_AtTheTop_IsListedFirst()
     {
+        // Profiles are updated and rendered in list order, the first category ends up on top
         ProfileCategory category = ProfileService.CreateProfileCategory("Tests", true);
 
-        // Pinned as-is: the category is created with order 1 but appended to the end of ProfileCategories. Profiles are
-        // updated and rendered in list order (the first category ends up on top), so until something saves a category,
-        // which sorts the list, this category's profiles are drawn underneath the others rather than on top
-        ProfileService.ProfileCategories.Last().ShouldBeSameAs(category);
-
-        ProfileService.SaveProfileCategory(General);
         ProfileService.ProfileCategories.First().ShouldBeSameAs(category);
     }
 
@@ -254,7 +249,7 @@ public class ProfileCategoryTests : ProfileServiceTestBase
     }
 
     [Fact]
-    public void CloneProfileConfiguration_SharesTheStoredDataWithTheOriginal()
+    public void CloneProfileConfiguration_HasItsOwnCopyOfTheStoredData()
     {
         ProfileConfiguration original = ProfileService.CreateProfileConfiguration(General, "Original", "TestTube");
 
@@ -267,12 +262,11 @@ public class ProfileCategoryTests : ProfileServiceTestBase
         // The clone is not added to the category
         General.ProfileConfigurations.ShouldBe([original]);
 
-        // Pinned as-is: documented as creating a copy, but the clone uses the same entity as the original, so saving
-        // changes made to the clone changes the original's stored data too
-        clone.Entity.ShouldBeSameAs(original.Entity);
+        clone.Entity.ShouldNotBeSameAs(original.Entity);
+        clone.Entity.Profile.ShouldNotBeSameAs(original.Entity.Profile);
         clone.Name = "Changed through the clone";
         clone.Save();
-        original.Entity.ProfileConfiguration.Name.ShouldBe("Changed through the clone");
+        original.Entity.ProfileConfiguration.Name.ShouldBe("Original");
     }
 
     [Fact]

src/Artemis.Core.Tests/Services/ProfileImportExportTests.cs

@@ -128,7 +128,7 @@ public class ProfileImportExportTests : ProfileServiceTestBase
     }
 
     [Fact]
-    public async Task ImportProfile_DoesNotRaiseProfileAdded()
+    public async Task ImportProfile_RaisesProfileAdded()
     {
         ProfileConfiguration original = CreateSavedProfile("Test profile");
         int profileAdded = 0;
@@ -138,9 +138,7 @@ public class ProfileImportExportTests : ProfileServiceTestBase
 
         await ImportExport(original, General, true);
 
-        // Pinned as-is: creating a profile raises ProfileAdded but importing one doesn't, only the category's own
-        // ProfileConfigurationAdded event tells listeners about an imported profile
-        profileAdded.ShouldBe(0);
+        profileAdded.ShouldBe(1);
         addedToCategory.ShouldBe(1);
     }
 
@@ -291,42 +289,41 @@ public class ProfileImportExportTests : ProfileServiceTestBase
     }
 
     [Fact]
-    public void ProfilesThatWereNeverSavedInTheEditor_AreStoredAsVersionZero()
+    public void NewProfiles_AreStoredAtTheCurrentVersion()
     {
         ProfileConfiguration configuration = ProfileService.CreateProfileConfiguration(General, "Test profile", "TestTube");
-        GetStoredProfiles().Single().ProfileConfiguration.Version.ShouldBe(0);
+        int version = GetStoredProfiles().Single().ProfileConfiguration.Version;
+        version.ShouldBeGreaterThan(0);
 
         ProfileService.SaveProfile(ProfileService.ActivateProfile(configuration), true);
 
-        GetStoredProfiles().Single().ProfileConfiguration.Version.ShouldBeGreaterThan(0);
+        GetStoredProfiles().Single().ProfileConfiguration.Version.ShouldBe(version);
     }
 
     [Fact]
-    public void ProfilesThatWereNeverSavedInTheEditor_FailToMigrateOnStartupButStillLoad()
+    public void NewProfiles_AreLeftAloneByTheMigrationsOnStartup()
     {
         ProfileService.CreateProfileConfiguration(General, "Test profile", "TestTube");
+        int version = GetStoredProfiles().Single().ProfileConfiguration.Version;
 
-        // Artemis migrates stored profiles once on startup, a failure is logged and the profile is left as it is
+        // Artemis migrates stored profiles once on startup
         Resolve<IProfileRepository>().MigrateProfiles();
 
-        GetStoredProfiles().Single().ProfileConfiguration.Version.ShouldBe(0);
+        GetStoredProfiles().Single().ProfileConfiguration.Version.ShouldBe(version);
         CreateNewProfileService().ProfileCategories.Single(c => c.Name == "General").ProfileConfigurations.Single().Name.ShouldBe("Test profile");
     }
 
     [Fact]
-    public async Task ImportProfile_OfAProfileThatWasNeverSavedInTheEditor_Fails()
+    public async Task ImportProfile_OfAProfileThatWasNeverSavedInTheEditor_Works()
     {
+        // Duplicating a freshly created profile from the sidebar or the editor is an export followed by an import
         ProfileConfiguration configuration = ProfileService.CreateProfileConfiguration(General, "Test profile", "TestTube");
         await using Stream export = await ProfileService.ExportProfile(configuration);
 
-        // Pinned as-is: saving the category (which is all creating a profile does) stores the profile without setting its
-        // migration version, only SaveProfile sets it. The export says version 0, so importing runs every migration on
-        // JSON that is already up to date and the first one fails on it. Duplicating a freshly created profile from the
-        // sidebar or the editor (an export followed by an import) runs into this. Restarting Artemis runs into it too, but
-        // there the failed migration is only logged and the profile loads normally, see the test above
-        InvalidOperationException exception = await Should.ThrowAsync<InvalidOperationException>(() => ProfileService.ImportProfile(export, General, true, false, "copy"));
-        exception.StackTrace!.ShouldContain("M0001NodeProviders");
-        General.ProfileConfigurations.ShouldBe([configuration]);
+        ProfileConfiguration imported = await ProfileService.ImportProfile(export, General, true, false, "copy");
+
+        imported.Name.ShouldBe("Test profile - copy");
+        General.ProfileConfigurations.Count.ShouldBe(2);
     }
 
     /// <summary>

src/Artemis.Core.Tests/Services/SettingsServiceTests.cs

@@ -75,15 +75,13 @@ public class SettingsServiceTests : IDisposable
     }
 
     [Fact]
-    public void GetSetting_AStoredValueThatDoesNotFitTheType_UsesTheDefaultOfTheType()
+    public void GetSetting_AStoredValueThatDoesNotFitTheType_UsesTheDefaultValuePassedIn()
     {
         Settings.GetSetting("Tests.Value", "not a number");
 
-        // Pinned as-is: when the stored value can't be read as the requested type the setting falls back to default(T)
-        // rather than to the default value passed in, so the caller's default of 7 is ignored here
         PluginSetting<int> setting = CreateNewSettingsService().GetSetting("Tests.Value", 7);
 
-        setting.Value.ShouldBe(0);
+        setting.Value.ShouldBe(7);
         setting.HasChanged.ShouldBeTrue();
     }
 

src/Artemis.Core.Tests/Storage/Migrations/M0001NodeProvidersTests.cs

@@ -112,7 +112,7 @@ public class M0001NodeProvidersTests
     }
 
     [Fact]
-    public void Migrate_NodeWithoutType_GetsANullType()
+    public void Migrate_NodeWithoutType_LeavesTheTypeOut()
     {
         JsonObject profile = OldProfileJson.Parse(OldProfileJson.Version0Profile);
         JsonObject node = profile.FindNode(OldProfileJson.LayerTransformNodeId);
@@ -120,10 +120,7 @@ public class M0001NodeProvidersTests
 
         _migration.Migrate(new JsonObject(), profile);
 
-        // Pinned as-is: a missing type is written back as "Type": null rather than left out, which gives the NodeEntity
-        // a null type even though the property isn't nullable
-        node.ContainsKey("Type").ShouldBeTrue();
-        node["Type"].ShouldBeNull();
+        node.ContainsKey("Type").ShouldBeFalse();
         node["ProviderId"]!.GetValue<string>().ShouldBe(OldProfileJson.GeneralNodesProviderId);
     }
 

src/Artemis.Core.Tests/Storage/Migrations/M0003SystemTextJsonTests.cs

@@ -115,7 +115,7 @@ public class M0003SystemTextJsonTests
     }
 
     [Fact]
-    public void Migrate_ListDirectlyInsideAList_KeepsItsValuesWrapper()
+    public void Migrate_ListDirectlyInsideAList_LosesItsValuesWrapperToo()
     {
         JsonObject profile = OldProfileJson.Parse("""
                                                   {"Rows": {"$type": "List", "$values": [{"$type": "List", "$values": [1, 2]}]}}
@@ -123,9 +123,7 @@ public class M0003SystemTextJsonTests
 
         _migration.Migrate(new JsonObject(), profile);
 
-        // Pinned as-is: items of a list are only searched for lists in their properties, a list that is itself an item
-        // keeps its "$values" wrapper. No profile entity has a list of lists, so old profiles aren't affected.
-        profile.ShouldBeJson("""{"Rows": [{"$values": [1, 2]}]}""");
+        profile.ShouldBeJson("""{"Rows": [[1, 2]]}""");
     }
 
     [Fact]

src/Artemis.Core.Tests/Storage/Migrations/M0004NodeStorageTests.cs

@@ -122,7 +122,7 @@ public class M0004NodeStorageTests
     }
 
     [Fact]
-    public void Migrate_NodeWithoutStorage_GetsANullStorage()
+    public void Migrate_NodeWithoutStorage_LeavesTheStorageOut()
     {
         (JsonObject configuration, JsonObject profile) = Version3Profile();
         JsonObject node = profile.FindNode(OldProfileJson.LayerTransformNodeId);
@@ -130,10 +130,7 @@ public class M0004NodeStorageTests
 
         _migration.Migrate(configuration, profile);
 
-        // Pinned as-is: a missing storage is written back as "Storage": null rather than left out, which gives the
-        // NodeEntity a null storage even though the property isn't nullable
-        node.ContainsKey("Storage").ShouldBeTrue();
-        node["Storage"].ShouldBeNull();
+        node.ContainsKey("Storage").ShouldBeFalse();
     }
 
     [Fact]

src/Artemis.Core.Tests/Storage/ProfileMigrationTests.cs

@@ -147,8 +147,8 @@ public class ProfileMigrationTests : IDisposable
         configuration.Remove("Version");
         JsonObject profile = OldProfileJson.Parse(OldProfileJson.MigratedProfile);
 
-        // Pinned as-is: without a version the profile is taken to be from before version 1 and M0001 fails on the
-        // current format, so a profile in the current format without a version can't be imported
+        // Without a version the profile is taken to be from before version 1, the version was added to tell those apart.
+        // M0001 fails on the current format, so a profile in the current format without a version can't be imported
         Should.Throw<InvalidOperationException>(() => _services.Resolve<IProfileRepository>().MigrateProfile(configuration, profile));
     }
 

src/Artemis.Core.Tests/Storage/Repositories/ProfileCategoryRepositoryTests.cs

@@ -117,15 +117,13 @@ public class ProfileCategoryRepositoryTests : IDisposable
     }
 
     [Fact]
-    public void IsUnique_ReturnsWhetherTheNameIsTaken()
+    public void IsUnique_ReturnsWhetherTheNameIsFree()
     {
         ProfileCategoryEntity games = new() {Name = "Games"};
         _repository.Add(games);
 
-        // Pinned as-is: despite its name this returns true when another category already has the name, so true means
-        // the name is not unique. Nothing in Artemis calls it at the moment.
-        _repository.IsUnique("Games", null).ShouldBeTrue();
-        _repository.IsUnique("Other", null).ShouldBeFalse();
+        _repository.IsUnique("Games", null).ShouldBeFalse();
+        _repository.IsUnique("Other", null).ShouldBeTrue();
     }
 
     [Fact]
@@ -134,8 +132,8 @@ public class ProfileCategoryRepositoryTests : IDisposable
         ProfileCategoryEntity games = new() {Name = "Games"};
         _repository.Add(games);
 
-        _repository.IsUnique("Games", games.Id).ShouldBeFalse();
-        _repository.IsUnique("Games", Guid.NewGuid()).ShouldBeTrue();
+        _repository.IsUnique("Games", games.Id).ShouldBeTrue();
+        _repository.IsUnique("Games", Guid.NewGuid()).ShouldBeFalse();
     }
 
     [Fact]
@@ -143,8 +141,8 @@ public class ProfileCategoryRepositoryTests : IDisposable
     {
         _repository.Add(new ProfileCategoryEntity {Name = "Games"});
 
-        _repository.IsUnique("  Games ", null).ShouldBeTrue();
-        _repository.IsUnique("games", null).ShouldBeFalse();
+        _repository.IsUnique("  Games ", null).ShouldBeFalse();
+        _repository.IsUnique("games", null).ShouldBeTrue();
     }
 
     [Fact]

src/Artemis.Core.Tests/Storage/Repositories/ProfileRepositoryTests.cs

@@ -5,7 +5,6 @@ using Artemis.Core.Tests.Infrastructure.Storage;
 using Artemis.Storage.Entities.Profile;
 using Artemis.Storage.Repositories;
 using Artemis.Storage.Repositories.Interfaces;
-using Microsoft.Data.Sqlite;
 using Microsoft.EntityFrameworkCore;
 
 namespace Artemis.Core.Tests.Storage.Repositories;
@@ -138,17 +137,17 @@ public class ProfileRepositoryTests : IDisposable
     }
 
     [Fact]
-    public void MigrateProfiles_ProfileWithoutVersion_IsNotMigrated()
+    public void MigrateProfiles_ProfileWithoutVersion_IsMigratedFromVersion0()
     {
         ProfileCategoryEntity category = StoredProfiles.AddCategory(_services.CreateDbContext);
         Guid id = StoredProfiles.InsertRaw(_services.CreateDbContext, category.Id, OldProfileJson.Version0Configuration, OldProfileJson.Version0Profile);
 
         _repository.MigrateProfiles();
 
-        // Pinned as-is: the database query only picks profiles whose version is below the latest, a missing version
-        // doesn't count as lower. MigrateProfile itself treats a missing version as version 0. Profiles stored by
-        // Entity Framework always have a version, so this only matters for rows written some other way.
-        StoredProfiles.ReadRaw(_services.CreateDbContext, id).ShouldBe((OldProfileJson.Version0Configuration, OldProfileJson.Version0Profile));
+        // Like MigrateProfile does, a missing version counts as version 0
+        (string configuration, string profile) = StoredProfiles.ReadRaw(_services.CreateDbContext, id);
+        JsonNode.Parse(configuration)!["Version"]!.GetValue<int>().ShouldBeGreaterThan(0);
+        profile.ShouldNotBe(OldProfileJson.Version0Profile);
     }
 
     [Theory]
@@ -169,38 +168,35 @@ public class ProfileRepositoryTests : IDisposable
     }
 
     [Fact]
-    public void MigrateProfiles_ConfigurationThatIsNotJson_StopsAllMigrations()
+    public void MigrateProfiles_ConfigurationThatIsNotJson_IsLoggedAndSkipped()
     {
         ProfileCategoryEntity category = StoredProfiles.AddCategory(_services.CreateDbContext);
         StoredProfiles.InsertRaw(_services.CreateDbContext, category.Id, "{broken", OldProfileJson.Version0Profile);
         Guid working = StoredProfiles.InsertRaw(_services.CreateDbContext, category.Id, StoredProfiles.Version0ConfigurationWithVersion(), OldProfileJson.Version0Profile);
 
-        // Pinned as-is: the version is read from the configuration by the database query, which fails on the broken
-        // JSON before any profile is migrated. Unlike a broken profile, this isn't caught and logged.
-        Should.Throw<SqliteException>(() => _repository.MigrateProfiles()).Message.ShouldContain("malformed JSON");
-        StoredProfiles.ReadRaw(_services.CreateDbContext, working).Profile.ShouldBe(OldProfileJson.Version0Profile);
+        _repository.MigrateProfiles();
+
+        _log.Errors.ShouldHaveSingleItem().MessageTemplate.Text.ShouldBe("Failed to migrate profile container {Id}");
+        StoredProfiles.ReadRaw(_services.CreateDbContext, working).Profile.ShouldNotBe(OldProfileJson.Version0Profile);
     }
 
     [Fact]
-    public void MigrateProfiles_ProfileCreatedInArtemis_IsStoredAtVersion0AndFailsToMigrate()
+    public void MigrateProfiles_ProfileCreatedInArtemis_IsStoredAtTheCurrentVersion()
     {
         IProfileService profileService = _services.Resolve<IProfileService>();
         ProfileCategory category = profileService.CreateProfileCategory("Created");
         profileService.CreateProfileConfiguration(category, "New profile", "Keyboard");
 
-        // Pinned as-is: a new profile is stored by saving its category, which doesn't set the version like
-        // ProfileRepository.Save does. The profile is in the current format, but at version 0 M0001 runs on it and
-        // fails. The error is logged each time the profiles are migrated (when the profile service starts) until the
-        // profile is saved by itself.
+        int version;
         using (var context = _services.CreateDbContext())
-            context.ProfileContainers.Single().ProfileConfiguration.Version.ShouldBe(0);
+            version = context.ProfileContainers.Single().ProfileConfiguration.Version;
+        version.ShouldBeGreaterThan(0);
 
         _repository.MigrateProfiles();
 
-        _log.Errors.ShouldHaveSingleItem().MessageTemplate.Text.ShouldBe("Failed to migrate profile container {Id}");
-        _log.Errors.Single().Exception.ShouldBeOfType<InvalidOperationException>();
+        _log.Errors.ShouldBeEmpty();
         using (var context = _services.CreateDbContext())
-            context.ProfileContainers.Single().ProfileConfiguration.Version.ShouldBe(0);
+            context.ProfileContainers.Single().ProfileConfiguration.Version.ShouldBe(version);
     }
 
     private static ProfileContainerEntity CreateContainer(ProfileCategoryEntity category, string name = "Profile")

src/Artemis.Core/Models/Surface/Layout/ArtemisLayout.cs

@@ -164,8 +164,8 @@ public class ArtemisLayout
             if (device.Leds.Count == 1)
                 return new ArtemisLayout(Path.Combine(DefaultLayoutPath, "Headset", "Artemis 1 LED headset.xml"));
             if (device.Leds.Count == 2)
-                return new ArtemisLayout(Path.Combine(DefaultLayoutPath, "Headset", "Artemis 2 LED headset.xml"));
-            return new ArtemisLayout(Path.Combine(DefaultLayoutPath, "Headset", "Artemis 4 LED headset.xml"));
+                return new ArtemisLayout(Path.Combine(DefaultLayoutPath, "Headset", "Artemis 2 LEDs headset.xml"));
+            return new ArtemisLayout(Path.Combine(DefaultLayoutPath, "Headset", "Artemis 4 LEDs headset.xml"));
         }
 
         return null;

src/Artemis.Core/Plugins/Settings/PluginSetting.cs

@@ -15,7 +15,7 @@ public class PluginSetting<T> : CorePropertyChanged, IPluginSetting
     private readonly PluginSettingEntity _pluginSettingEntity;
     private T _value;
 
-    internal PluginSetting(IPluginRepository pluginRepository, PluginSettingEntity pluginSettingEntity)
+    internal PluginSetting(IPluginRepository pluginRepository, PluginSettingEntity pluginSettingEntity, T? defaultValue = default)
     {
         _pluginRepository = pluginRepository;
         _pluginSettingEntity = pluginSettingEntity;
@@ -27,7 +27,7 @@ public class PluginSetting<T> : CorePropertyChanged, IPluginSetting
         }
         catch (JsonException)
         {
-            _value = default!;
+            _value = defaultValue!;
         }
     }
 

src/Artemis.Core/Plugins/Settings/PluginSettings.cs

@@ -32,7 +32,7 @@ public class PluginSettings
     /// </summary>
     /// <typeparam name="T">The type of the setting, can be any serializable type</typeparam>
     /// <param name="name">The name of the setting, may not be longer than 128 characters</param>
-    /// <param name="defaultValue">The default value to use if the setting does not exist yet</param>
+    /// <param name="defaultValue">The default value to use if the setting does not exist yet, or can't be read as <typeparamref name="T" /></param>
     public PluginSetting<T> GetSetting<T>(string name, T? defaultValue = default)
     {
         if (name.Length > 128)
@@ -57,7 +57,7 @@ public class PluginSettings
                 _pluginRepository.SaveSetting(settingEntity);
             }
 
-            PluginSetting<T> pluginSetting = new(_pluginRepository, settingEntity);
+            PluginSetting<T> pluginSetting = new(_pluginRepository, settingEntity, defaultValue);
 
             // This overrides null with the default value, I'm not sure if that's desirable because you
             // might expect something to go null and you might not

src/Artemis.Core/Services/Storage/Models/SurfaceArrangementType.cs

@@ -54,6 +54,8 @@ internal class SurfaceArrangementType
         ) {SurfaceArrangement = SurfaceArrangement};
         fallback.Apply(devicesToArrange, devices);
         AppliedConfiguration = fallback;
+        foreach (ArtemisDevice artemisDevice in devicesToArrange)
+            artemisDevice.ZIndex = ZIndex;
     }
 
     public Point GetEdge(HorizontalArrangementPosition horizontalPosition, VerticalArrangementPosition verticalPosition)

src/Artemis.Core/Services/Storage/ProfileService.cs

@@ -161,7 +161,16 @@ internal class ProfileService : IProfileService
     /// <inheritdoc />
     public ProfileConfiguration CloneProfileConfiguration(ProfileConfiguration profileConfiguration)
     {
-        return new ProfileConfiguration(profileConfiguration.Category, profileConfiguration.Entity);
+        ProfileContainerEntity original = profileConfiguration.Entity;
+        ProfileContainerEntity copy = new()
+        {
+            Id = original.Id,
+            Icon = original.Icon.ToArray(),
+            ProfileCategory = original.ProfileCategory,
+            ProfileConfiguration = CopyEntity(original.ProfileConfiguration),
+            Profile = CopyEntity(original.Profile)
+        };
+        return new ProfileConfiguration(profileConfiguration.Category, copy);
     }
 
     /// <inheritdoc />
@@ -234,7 +243,7 @@ internal class ProfileService : IProfileService
         }
 
         _profileCategoryRepository.Add(profileCategory.Entity);
-        ProfileCategories = new ReadOnlyCollection<ProfileCategory>([..ProfileCategories, profileCategory]);
+        ProfileCategories = new ReadOnlyCollection<ProfileCategory>(ProfileCategories.Append(profileCategory).OrderBy(c => c.Order).ToList());
 
         OnProfileCategoryAdded(new ProfileCategoryEventArgs(profileCategory));
         return profileCategory;
@@ -259,6 +268,8 @@ internal class ProfileService : IProfileService
 
         category.AddProfileConfiguration(configuration, category.ProfileConfigurations.FirstOrDefault());
         SaveProfileCategory(category);
+        // The profile is in the current format, saving it stores the current migration version with it
+        _profileRepository.Save(configuration.Entity);
         
         OnProfileAdded(new ProfileConfigurationEventArgs(configuration));
         return configuration;
@@ -303,7 +314,18 @@ internal class ProfileService : IProfileService
         profile.IsFreshImport = false;
         profile.ProfileEntity.IsFreshImport = false;
 
-        _profileRepository.Save(profile.Configuration.Entity);
+        // A copy made by CloneProfileConfiguration has its own stored data, save it into that of the original
+        ProfileContainerEntity entity = profile.Configuration.Entity;
+        ProfileConfiguration? original = ProfileCategories
+            .SelectMany(c => c.ProfileConfigurations)
+            .FirstOrDefault(c => c.Entity != entity && c.Entity.Id == entity.Id);
+        if (original != null)
+        {
+            original.Entity.Profile = CopyEntity(entity.Profile);
+            entity = original.Entity;
+        }
+
+        _profileRepository.Save(entity);
 
         // If the provided profile is external (cloned or from the workshop?) but it is loaded locally too, reload the local instance
         // A bit dodge but it ensures local instances always represent the latest stored version
@@ -429,6 +451,7 @@ internal class ProfileService : IProfileService
         profileConfiguration.LoadModules(modules);
         SaveProfileCategory(category);
 
+        OnProfileAdded(new ProfileConfigurationEventArgs(profileConfiguration));
         return profileConfiguration;
     }
 
@@ -448,6 +471,11 @@ internal class ProfileService : IProfileService
         SaveProfileCategory(profile.Configuration.Category);
     }
 
+    private static T CopyEntity<T>(T entity)
+    {
+        return CoreJson.Deserialize<T>(CoreJson.Serialize(entity))!;
+    }
+
     private void InputServiceOnKeyboardKeyUp(object? sender, ArtemisKeyboardKeyEventArgs e)
     {
         _pendingKeyboardEvents.Add(e);

src/Artemis.Storage/Migrations/Profile/M0001NodeProviders.cs

@@ -83,7 +83,8 @@ namespace Artemis.Storage.Migrations.Profile
                 if (jsonNode == null)
                     continue;
                 JsonObject nodeObject = jsonNode.AsObject();
-                nodeObject["Type"] = nodeObject["Type"]?.GetValue<string>().Replace("Artemis.VisualScripting.Nodes", "Artemis.Plugins.Nodes.General.Nodes");
+                if (nodeObject["Type"] != null)
+                    nodeObject["Type"] = nodeObject["Type"]!.GetValue<string>().Replace("Artemis.VisualScripting.Nodes", "Artemis.Plugins.Nodes.General.Nodes");
                 nodeObject["ProviderId"] = "Artemis.Plugins.Nodes.General.GeneralNodesProvider-d9e1ee78";
             }
         }

src/Artemis.Storage/Migrations/Profile/M0003SystemTextJson.cs

@@ -36,11 +36,7 @@ internal class M0003SystemTextJson : IProfileMigration
                 {
                     obj.Remove("$values");
                     jsonObject[key] = values;
-                    foreach (JsonNode? jsonNode in values.ToList())
-                    {
-                        if (jsonNode is JsonObject childObject)
-                            ConvertToSystemTextJson(childObject);
-                    }
+                    ConvertListItems(values);
                 }
 
                 obj.Remove("$type");
@@ -52,6 +48,27 @@ internal class M0003SystemTextJson : IProfileMigration
         }
     }
 
+    private static void ConvertListItems(JsonArray values)
+    {
+        for (int i = 0; i < values.Count; i++)
+        {
+            if (values[i] is not JsonObject childObject)
+                continue;
+
+            // A list that is itself an item of a list is wrapped too
+            if (childObject["$type"] != null && childObject["$values"] is JsonArray childValues)
+            {
+                childObject.Remove("$values");
+                values[i] = childValues;
+                ConvertListItems(childValues);
+            }
+            else
+            {
+                ConvertToSystemTextJson(childObject);
+            }
+        }
+    }
+
     internal static void FilterType(JsonObject jsonObject)
     {
         // Replace or remove $type depending on whether there's a matching JsonDerivedType

src/Artemis.Storage/Migrations/Profile/M0004NodeStorage.cs

@@ -97,7 +97,8 @@ namespace Artemis.Storage.Migrations.Profile
                     continue;
                 
                 JsonObject nodeObject = jsonNode.AsObject();
-                nodeObject["Storage"] = MigrateNodeStorageJson(nodeObject["Storage"]?.GetValue<string>(), _logger);
+                if (nodeObject["Storage"] != null)
+                    nodeObject["Storage"] = MigrateNodeStorageJson(nodeObject["Storage"]!.GetValue<string>(), _logger);
             }
         }
         

src/Artemis.Storage/Repositories/ProfileCategoryRepository.cs

@@ -69,7 +69,7 @@ internal class ProfileCategoryRepository(Func<ArtemisDbContext> getContext, IPro
 
         name = name.Trim();
         return id == null
-            ? dbContext.ProfileCategories.Any(p => p.Name == name)
-            : dbContext.ProfileCategories.Any(p => p.Name == name && p.Id != id.Value);
+            ? !dbContext.ProfileCategories.Any(p => p.Name == name)
+            : !dbContext.ProfileCategories.Any(p => p.Name == name && p.Id != id.Value);
     }
 }

src/Artemis.Storage/Repositories/ProfileRepository.cs

@@ -51,9 +51,13 @@ public class ProfileRepository(ILogger logger, Func<ArtemisDbContext> getContext
         using ArtemisDbContext dbContext = getContext();
         int max = profileMigrators.Max(m => m.Version);
 
-        // Query the ProfileContainerEntity table directly, grabbing the ID, profile, and configuration
+        // Query the ProfileContainerEntity table directly, grabbing the ID, profile, and configuration.
+        // A missing version counts as version 0, and configurations that aren't valid JSON are included so that they fail
+        // and get logged below rather than failing the query
         List<RawProfileContainer> containers = dbContext.Database
-            .SqlQueryRaw<RawProfileContainer>("SELECT Id, Profile, ProfileConfiguration FROM ProfileContainers WHERE json_extract(ProfileConfiguration, '$.Version') < {0}", max)
+            .SqlQueryRaw<RawProfileContainer>(
+                "SELECT Id, Profile, ProfileConfiguration FROM ProfileContainers WHERE CASE WHEN json_valid(ProfileConfiguration) THEN COALESCE(json_extract(ProfileConfiguration, '$.Version'), 0) < {0} ELSE 1 END",
+                max)
             .ToList();
 
         foreach (RawProfileContainer rawProfileContainer in containers)