diff --git a/src/Artemis.Core.Tests/Services/DeviceServiceTests.cs b/src/Artemis.Core.Tests/Services/DeviceServiceTests.cs index 2de585117..3e7ab284c 100644 --- a/src/Artemis.Core.Tests/Services/DeviceServiceTests.cs +++ b/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] diff --git a/src/Artemis.Core.Tests/Services/ProfileCategoryTests.cs b/src/Artemis.Core.Tests/Services/ProfileCategoryTests.cs index bf1f859d8..7284e2e00 100644 --- a/src/Artemis.Core.Tests/Services/ProfileCategoryTests.cs +++ b/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] diff --git a/src/Artemis.Core.Tests/Services/ProfileImportExportTests.cs b/src/Artemis.Core.Tests/Services/ProfileImportExportTests.cs index a82545156..dd31170ad 100644 --- a/src/Artemis.Core.Tests/Services/ProfileImportExportTests.cs +++ b/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().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(() => 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); } /// diff --git a/src/Artemis.Core.Tests/Services/SettingsServiceTests.cs b/src/Artemis.Core.Tests/Services/SettingsServiceTests.cs index 61db1621a..5ac449802 100644 --- a/src/Artemis.Core.Tests/Services/SettingsServiceTests.cs +++ b/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 setting = CreateNewSettingsService().GetSetting("Tests.Value", 7); - setting.Value.ShouldBe(0); + setting.Value.ShouldBe(7); setting.HasChanged.ShouldBeTrue(); } diff --git a/src/Artemis.Core.Tests/Storage/Migrations/M0001NodeProvidersTests.cs b/src/Artemis.Core.Tests/Storage/Migrations/M0001NodeProvidersTests.cs index 19708aa40..4fcea71b6 100644 --- a/src/Artemis.Core.Tests/Storage/Migrations/M0001NodeProvidersTests.cs +++ b/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().ShouldBe(OldProfileJson.GeneralNodesProviderId); } diff --git a/src/Artemis.Core.Tests/Storage/Migrations/M0003SystemTextJsonTests.cs b/src/Artemis.Core.Tests/Storage/Migrations/M0003SystemTextJsonTests.cs index 90dfb9441..bde9b6e29 100644 --- a/src/Artemis.Core.Tests/Storage/Migrations/M0003SystemTextJsonTests.cs +++ b/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] diff --git a/src/Artemis.Core.Tests/Storage/Migrations/M0004NodeStorageTests.cs b/src/Artemis.Core.Tests/Storage/Migrations/M0004NodeStorageTests.cs index 1e5fe3d9c..9ed289b3e 100644 --- a/src/Artemis.Core.Tests/Storage/Migrations/M0004NodeStorageTests.cs +++ b/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] diff --git a/src/Artemis.Core.Tests/Storage/ProfileMigrationTests.cs b/src/Artemis.Core.Tests/Storage/ProfileMigrationTests.cs index 04cca65ec..765b4ea55 100644 --- a/src/Artemis.Core.Tests/Storage/ProfileMigrationTests.cs +++ b/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(() => _services.Resolve().MigrateProfile(configuration, profile)); } diff --git a/src/Artemis.Core.Tests/Storage/Repositories/ProfileCategoryRepositoryTests.cs b/src/Artemis.Core.Tests/Storage/Repositories/ProfileCategoryRepositoryTests.cs index 351311872..b138da93d 100644 --- a/src/Artemis.Core.Tests/Storage/Repositories/ProfileCategoryRepositoryTests.cs +++ b/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] diff --git a/src/Artemis.Core.Tests/Storage/Repositories/ProfileRepositoryTests.cs b/src/Artemis.Core.Tests/Storage/Repositories/ProfileRepositoryTests.cs index 1319c0cbd..5b49549d0 100644 --- a/src/Artemis.Core.Tests/Storage/Repositories/ProfileRepositoryTests.cs +++ b/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().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(() => _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(); 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(); + _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") diff --git a/src/Artemis.Core/Models/Surface/Layout/ArtemisLayout.cs b/src/Artemis.Core/Models/Surface/Layout/ArtemisLayout.cs index 20309d906..a92b9c144 100644 --- a/src/Artemis.Core/Models/Surface/Layout/ArtemisLayout.cs +++ b/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; diff --git a/src/Artemis.Core/Plugins/Settings/PluginSetting.cs b/src/Artemis.Core/Plugins/Settings/PluginSetting.cs index 8f14d542f..7eb1abbc4 100644 --- a/src/Artemis.Core/Plugins/Settings/PluginSetting.cs +++ b/src/Artemis.Core/Plugins/Settings/PluginSetting.cs @@ -15,7 +15,7 @@ public class PluginSetting : 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 : CorePropertyChanged, IPluginSetting } catch (JsonException) { - _value = default!; + _value = defaultValue!; } } diff --git a/src/Artemis.Core/Plugins/Settings/PluginSettings.cs b/src/Artemis.Core/Plugins/Settings/PluginSettings.cs index 811dcf817..34f126910 100644 --- a/src/Artemis.Core/Plugins/Settings/PluginSettings.cs +++ b/src/Artemis.Core/Plugins/Settings/PluginSettings.cs @@ -32,7 +32,7 @@ public class PluginSettings /// /// The type of the setting, can be any serializable type /// The name of the setting, may not be longer than 128 characters - /// The default value to use if the setting does not exist yet + /// The default value to use if the setting does not exist yet, or can't be read as public PluginSetting GetSetting(string name, T? defaultValue = default) { if (name.Length > 128) @@ -57,7 +57,7 @@ public class PluginSettings _pluginRepository.SaveSetting(settingEntity); } - PluginSetting pluginSetting = new(_pluginRepository, settingEntity); + PluginSetting 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 diff --git a/src/Artemis.Core/Services/Storage/Models/SurfaceArrangementType.cs b/src/Artemis.Core/Services/Storage/Models/SurfaceArrangementType.cs index b2a3e24d8..761648ee0 100644 --- a/src/Artemis.Core/Services/Storage/Models/SurfaceArrangementType.cs +++ b/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) diff --git a/src/Artemis.Core/Services/Storage/ProfileService.cs b/src/Artemis.Core/Services/Storage/ProfileService.cs index a712c4ae7..0e6a1d256 100644 --- a/src/Artemis.Core/Services/Storage/ProfileService.cs +++ b/src/Artemis.Core/Services/Storage/ProfileService.cs @@ -161,7 +161,16 @@ internal class ProfileService : IProfileService /// 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); } /// @@ -234,7 +243,7 @@ internal class ProfileService : IProfileService } _profileCategoryRepository.Add(profileCategory.Entity); - ProfileCategories = new ReadOnlyCollection([..ProfileCategories, profileCategory]); + ProfileCategories = new ReadOnlyCollection(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 entity) + { + return CoreJson.Deserialize(CoreJson.Serialize(entity))!; + } + private void InputServiceOnKeyboardKeyUp(object? sender, ArtemisKeyboardKeyEventArgs e) { _pendingKeyboardEvents.Add(e); diff --git a/src/Artemis.Storage/Migrations/Profile/M0001NodeProviders.cs b/src/Artemis.Storage/Migrations/Profile/M0001NodeProviders.cs index 701ab91ee..d290d0750 100644 --- a/src/Artemis.Storage/Migrations/Profile/M0001NodeProviders.cs +++ b/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().Replace("Artemis.VisualScripting.Nodes", "Artemis.Plugins.Nodes.General.Nodes"); + if (nodeObject["Type"] != null) + nodeObject["Type"] = nodeObject["Type"]!.GetValue().Replace("Artemis.VisualScripting.Nodes", "Artemis.Plugins.Nodes.General.Nodes"); nodeObject["ProviderId"] = "Artemis.Plugins.Nodes.General.GeneralNodesProvider-d9e1ee78"; } } diff --git a/src/Artemis.Storage/Migrations/Profile/M0003SystemTextJson.cs b/src/Artemis.Storage/Migrations/Profile/M0003SystemTextJson.cs index 47771f2cb..04f0e4410 100644 --- a/src/Artemis.Storage/Migrations/Profile/M0003SystemTextJson.cs +++ b/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 diff --git a/src/Artemis.Storage/Migrations/Profile/M0004NodeStorage.cs b/src/Artemis.Storage/Migrations/Profile/M0004NodeStorage.cs index f82ad9b98..aeb762827 100644 --- a/src/Artemis.Storage/Migrations/Profile/M0004NodeStorage.cs +++ b/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(), _logger); + if (nodeObject["Storage"] != null) + nodeObject["Storage"] = MigrateNodeStorageJson(nodeObject["Storage"]!.GetValue(), _logger); } } diff --git a/src/Artemis.Storage/Repositories/ProfileCategoryRepository.cs b/src/Artemis.Storage/Repositories/ProfileCategoryRepository.cs index 5ad5ae4d7..b25ef21c2 100644 --- a/src/Artemis.Storage/Repositories/ProfileCategoryRepository.cs +++ b/src/Artemis.Storage/Repositories/ProfileCategoryRepository.cs @@ -69,7 +69,7 @@ internal class ProfileCategoryRepository(Func 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); } } \ No newline at end of file diff --git a/src/Artemis.Storage/Repositories/ProfileRepository.cs b/src/Artemis.Storage/Repositories/ProfileRepository.cs index 28d273aca..12fb48dfc 100644 --- a/src/Artemis.Storage/Repositories/ProfileRepository.cs +++ b/src/Artemis.Storage/Repositories/ProfileRepository.cs @@ -51,9 +51,13 @@ public class ProfileRepository(ILogger logger, Func 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 containers = dbContext.Database - .SqlQueryRaw("SELECT Id, Profile, ProfileConfiguration FROM ProfileContainers WHERE json_extract(ProfileConfiguration, '$.Version') < {0}", max) + .SqlQueryRaw( + "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)