Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions Assets/Tests/Editor/OnionAssemblyDependencyTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -206,18 +206,21 @@ public void ScriptChangesDuringPlayOptions_WhenLoaded_CompilesUnderDomainAssembl
}

[Test]
public void ToolSettings_WhenLoaded_PinPortsToDomainAndStorageDtoToInfrastructure()
public void Settings_WhenLoaded_PinPortsToDomainAndStorageDtosToInfrastructure()
{
// Tests that tool-specific settings ports stay in the domain layer while JSON storage DTOs stay in infrastructure.
// Tests that settings ports and value objects stay in the domain layer while JSON storage DTOs stay in infrastructure.
string editorSettingsDataAssemblyName = typeof(UnityCliLoopEditorSettingsData).Assembly.GetName().Name;
string toolSettingsPortAssemblyName = typeof(IToolSettingsPort).Assembly.GetName().Name;
string editorSettingsPortAssemblyName = typeof(IUnityCliLoopEditorSettingsPort).Assembly.GetName().Name;
string toolSettingsDataAssemblyName = typeof(ToolSettingsData).Assembly.GetName().Name;
string editorSettingsJsonDataAssemblyName =
typeof(UnityCliLoopEditorSettingsJsonData).Assembly.GetName().Name;

Assert.That(editorSettingsDataAssemblyName, Is.EqualTo(DomainAssemblyName));
Assert.That(toolSettingsPortAssemblyName, Is.EqualTo(DomainAssemblyName));
Assert.That(editorSettingsPortAssemblyName, Is.EqualTo(DomainAssemblyName));
Assert.That(toolSettingsDataAssemblyName, Is.EqualTo(InfrastructureAssemblyName));
Assert.That(editorSettingsJsonDataAssemblyName, Is.EqualTo(InfrastructureAssemblyName));
}

[Test]
Expand Down
59 changes: 41 additions & 18 deletions Assets/Tests/Editor/UnityCliLoopEditorSettingsRecoveryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,15 +84,14 @@ public void TearDown()
public void RecoverSettingsFileIfNeeded_WhenPrimaryMissingAndBackupExists_ShouldRestoreBackup()
{
UnityCliLoopEditorSettingsData backupData = new() { showDeveloperTools = true };
File.WriteAllText(BackupFilePath, JsonUtility.ToJson(backupData, true));
File.WriteAllText(BackupFilePath, SerializeSettingsData(backupData));

_editorSettingsPort.RecoverSettingsFileIfNeeded();

Assert.IsTrue(File.Exists(SettingsFilePath), "Primary settings file should be restored from backup");
Assert.IsFalse(File.Exists(BackupFilePath), "Backup should be consumed after recovery");

UnityCliLoopEditorSettingsData restored = JsonUtility.FromJson<UnityCliLoopEditorSettingsData>(
File.ReadAllText(SettingsFilePath));
UnityCliLoopEditorSettingsJsonData restored = DeserializeSettingsJson(File.ReadAllText(SettingsFilePath));
Assert.AreEqual(backupData.showDeveloperTools, restored.showDeveloperTools);
}

Expand All @@ -101,35 +100,33 @@ public void RecoverSettingsFileIfNeeded_WhenPrimaryMissingAndTempExists_ShouldPr
{
UnityCliLoopEditorSettingsData oldData = new() { showDeveloperTools = false };
UnityCliLoopEditorSettingsData newData = new() { showDeveloperTools = true };
File.WriteAllText(BackupFilePath, JsonUtility.ToJson(oldData, true));
File.WriteAllText(TempFilePath, JsonUtility.ToJson(newData, true));
File.WriteAllText(BackupFilePath, SerializeSettingsData(oldData));
File.WriteAllText(TempFilePath, SerializeSettingsData(newData));

_editorSettingsPort.RecoverSettingsFileIfNeeded();

Assert.IsTrue(File.Exists(SettingsFilePath), "Primary settings file should be restored from temp");
Assert.IsFalse(File.Exists(BackupFilePath), "Backup should be removed after temp recovery");
Assert.IsFalse(File.Exists(TempFilePath), "Temp file should be consumed after recovery");

UnityCliLoopEditorSettingsData restored = JsonUtility.FromJson<UnityCliLoopEditorSettingsData>(
File.ReadAllText(SettingsFilePath));
UnityCliLoopEditorSettingsJsonData restored = DeserializeSettingsJson(File.ReadAllText(SettingsFilePath));
Assert.AreEqual(newData.showDeveloperTools, restored.showDeveloperTools);
}

[Test]
public void RecoverSettingsFileIfNeeded_WhenPrimaryExists_ShouldCleanStaleSidecars()
{
UnityCliLoopEditorSettingsData primaryData = new() { showDeveloperTools = true };
File.WriteAllText(SettingsFilePath, JsonUtility.ToJson(primaryData, true));
File.WriteAllText(BackupFilePath, JsonUtility.ToJson(new UnityCliLoopEditorSettingsData { showDeveloperTools = false }, true));
File.WriteAllText(TempFilePath, JsonUtility.ToJson(new UnityCliLoopEditorSettingsData { showDeveloperTools = false }, true));
File.WriteAllText(SettingsFilePath, SerializeSettingsData(primaryData));
File.WriteAllText(BackupFilePath, SerializeSettingsData(new UnityCliLoopEditorSettingsData { showDeveloperTools = false }));
File.WriteAllText(TempFilePath, SerializeSettingsData(new UnityCliLoopEditorSettingsData { showDeveloperTools = false }));

_editorSettingsPort.RecoverSettingsFileIfNeeded();

Assert.IsFalse(File.Exists(BackupFilePath), "Backup should not linger once primary exists");
Assert.IsFalse(File.Exists(TempFilePath), "Temp should not linger once primary exists");

UnityCliLoopEditorSettingsData restored = JsonUtility.FromJson<UnityCliLoopEditorSettingsData>(
File.ReadAllText(SettingsFilePath));
UnityCliLoopEditorSettingsJsonData restored = DeserializeSettingsJson(File.ReadAllText(SettingsFilePath));
Assert.AreEqual(primaryData.showDeveloperTools, restored.showDeveloperTools);
}

Expand All @@ -142,7 +139,7 @@ public void GetSettings_WhenLegacySetupWizardStateExists_ShouldMigrateVersionSta
lastSeenSetupWizardVersion = "2.1.1",
suppressSetupWizardAutoShow = false
};
File.WriteAllText(LegacySettingsFilePath, JsonUtility.ToJson(legacyData, true));
File.WriteAllText(LegacySettingsFilePath, SerializeSettingsData(legacyData));

string lastSeenVersion = _editorSettingsPort.GetLastSeenSetupWizardVersion();
bool suppressAutoShow = _editorSettingsPort.GetSuppressSetupWizardAutoShow();
Expand All @@ -162,7 +159,7 @@ public void GetSettings_WhenLegacySetupWizardAutoShowWasSuppressed_ShouldMigrate
lastSeenSetupWizardVersion = "2.1.1",
suppressSetupWizardAutoShow = true
};
File.WriteAllText(LegacySettingsFilePath, JsonUtility.ToJson(legacyData, true));
File.WriteAllText(LegacySettingsFilePath, SerializeSettingsData(legacyData));

bool suppressAutoShow = _editorSettingsPort.GetSuppressSetupWizardAutoShow();

Expand All @@ -184,8 +181,8 @@ public void GetSettings_WhenCurrentSetupWizardVersionExistsBeforeMigration_Shoul
lastSeenSetupWizardVersion = "2.1.1",
suppressSetupWizardAutoShow = false
};
File.WriteAllText(SettingsFilePath, JsonUtility.ToJson(currentData, true));
File.WriteAllText(LegacySettingsFilePath, JsonUtility.ToJson(legacyData, true));
File.WriteAllText(SettingsFilePath, SerializeSettingsData(currentData));
File.WriteAllText(LegacySettingsFilePath, SerializeSettingsData(legacyData));

string lastSeenVersion = _editorSettingsPort.GetLastSeenSetupWizardVersion();
bool suppressAutoShow = _editorSettingsPort.GetSuppressSetupWizardAutoShow();
Expand All @@ -210,8 +207,8 @@ public void GetSettings_WhenLegacySetupWizardStateAlreadyMigrated_ShouldKeepCurr
lastSeenSetupWizardVersion = "2.1.1",
suppressSetupWizardAutoShow = true
};
File.WriteAllText(SettingsFilePath, JsonUtility.ToJson(currentData, true));
File.WriteAllText(LegacySettingsFilePath, JsonUtility.ToJson(legacyData, true));
File.WriteAllText(SettingsFilePath, SerializeSettingsData(currentData));
File.WriteAllText(LegacySettingsFilePath, SerializeSettingsData(legacyData));

string lastSeenVersion = _editorSettingsPort.GetLastSeenSetupWizardVersion();
bool suppressAutoShow = _editorSettingsPort.GetSuppressSetupWizardAutoShow();
Expand All @@ -232,6 +229,22 @@ public void GetInstallSkillsFlat_WhenMissingFromSettings_DefaultsToTrue()
Assert.IsTrue(installSkillsFlat);
}

[Test]
public void GetSettings_WhenStringFieldIsExplicitNull_ShouldPreserveJsonUtilityEmptyStringMaterializationAndApplyMissingDefaults()
{
// Verifies that DTO-to-VO conversion preserves JsonUtility's empty-string materialization.
File.WriteAllText(SettingsFilePath, "{\"lastSeenSetupWizardVersion\":null}");
_editorSettingsRepository.InvalidateCache();

UnityCliLoopEditorSettingsData settings = _editorSettingsPort.GetSettings();

Assert.That(settings.lastSeenSetupWizardVersion, Is.Empty);
Assert.That(settings.lastSeenSetupWizardMinimumDispatcherVersion, Is.Empty);
Assert.That(settings.showUnityCliLoopSecuritySetting, Is.True);
Assert.That(settings.showToolSettings, Is.True);
Assert.That(settings.installSkillsFlat, Is.True);
}

[Test]
public void RecoverSettingsFileIfNeeded_WhenLegacyPortFieldsExist_ShouldRemoveThem()
{
Expand Down Expand Up @@ -327,6 +340,16 @@ private static void DeleteIfExists(string path)
}
}

private static string SerializeSettingsData(UnityCliLoopEditorSettingsData settings)
{
return JsonUtility.ToJson(UnityCliLoopEditorSettingsJsonData.FromDomain(settings), true);
}

private static UnityCliLoopEditorSettingsJsonData DeserializeSettingsJson(string json)
{
return JsonUtility.FromJson<UnityCliLoopEditorSettingsJsonData>(json);
}

/// <summary>
/// Test support type used by editor and play mode fixtures.
/// </summary>
Expand Down
7 changes: 7 additions & 0 deletions Packages/src/Editor/Domain/IsExternalInit.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
namespace System.Runtime.CompilerServices
{
// Unity's .NET profile does not provide this marker type, but init accessors require it.
internal static class IsExternalInit
{
}
}
11 changes: 11 additions & 0 deletions Packages/src/Editor/Domain/IsExternalInit.cs.meta

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

19 changes: 8 additions & 11 deletions Packages/src/Editor/Domain/UnityCliLoopEditorSettingsData.cs
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
using System;

namespace io.github.hatayama.UnityCliLoop.Domain
{
[Serializable]
public record UnityCliLoopEditorSettingsData
{
public bool showDeveloperTools = false;
public string lastSeenSetupWizardVersion = "";
public string lastSeenSetupWizardMinimumDispatcherVersion = "";
public bool suppressSetupWizardAutoShow = false;
public bool legacySetupWizardStateMigrated = false;
public bool showUnityCliLoopSecuritySetting = true;
public bool showToolSettings = true;
public bool installSkillsFlat = true;
public bool showDeveloperTools { get; init; } = false;
public string lastSeenSetupWizardVersion { get; init; } = "";
public string lastSeenSetupWizardMinimumDispatcherVersion { get; init; } = "";
public bool suppressSetupWizardAutoShow { get; init; } = false;
public bool legacySetupWizardStateMigrated { get; init; } = false;
public bool showUnityCliLoopSecuritySetting { get; init; } = true;
public bool showToolSettings { get; init; } = true;
public bool installSkillsFlat { get; init; } = true;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
using System;
using System.Diagnostics;

using io.github.hatayama.UnityCliLoop.Domain;

namespace io.github.hatayama.UnityCliLoop.Infrastructure
{
/// <summary>
/// JsonUtility storage DTO for Unity CLI Loop editor settings.
/// </summary>
[Serializable]
internal sealed class UnityCliLoopEditorSettingsJsonData
{
public bool showDeveloperTools = false;
public string lastSeenSetupWizardVersion = "";
public string lastSeenSetupWizardMinimumDispatcherVersion = "";
public bool suppressSetupWizardAutoShow = false;
public bool legacySetupWizardStateMigrated = false;
public bool showUnityCliLoopSecuritySetting = true;
public bool showToolSettings = true;
public bool installSkillsFlat = true;

internal UnityCliLoopEditorSettingsData ToDomain()
{
return new UnityCliLoopEditorSettingsData
{
showDeveloperTools = showDeveloperTools,
lastSeenSetupWizardVersion = lastSeenSetupWizardVersion,
lastSeenSetupWizardMinimumDispatcherVersion = lastSeenSetupWizardMinimumDispatcherVersion,
suppressSetupWizardAutoShow = suppressSetupWizardAutoShow,
legacySetupWizardStateMigrated = legacySetupWizardStateMigrated,
showUnityCliLoopSecuritySetting = showUnityCliLoopSecuritySetting,
showToolSettings = showToolSettings,
installSkillsFlat = installSkillsFlat
};
}

internal static UnityCliLoopEditorSettingsJsonData FromDomain(UnityCliLoopEditorSettingsData settings)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: FromDomain is an internal method, so the ArgumentNullException throw after the Debug.Assert is redundant and adds unnecessary exception surface. The project convention for internal APIs is to enforce preconditions with Debug.Assert alone. Remove the if (settings == null) guard and let the assert serve as the contract check.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At Packages/src/Editor/Infrastructure/Settings/UnityCliLoopEditorSettingsJsonData.cs, line 38:

<comment>FromDomain is an internal method, so the ArgumentNullException throw after the Debug.Assert is redundant and adds unnecessary exception surface. The project convention for internal APIs is to enforce preconditions with Debug.Assert alone. Remove the `if (settings == null)` guard and let the assert serve as the contract check.</comment>

<file context>
@@ -0,0 +1,60 @@
+            };
+        }
+
+        internal static UnityCliLoopEditorSettingsJsonData FromDomain(UnityCliLoopEditorSettingsData settings)
+        {
+            Debug.Assert(settings != null, "settings must not be null");
</file context>

{
Debug.Assert(settings != null, "settings must not be null");

if (settings == null)
{
throw new ArgumentNullException(nameof(settings));
}

return new UnityCliLoopEditorSettingsJsonData
{
showDeveloperTools = settings.showDeveloperTools,
lastSeenSetupWizardVersion = settings.lastSeenSetupWizardVersion,
lastSeenSetupWizardMinimumDispatcherVersion = settings.lastSeenSetupWizardMinimumDispatcherVersion,
suppressSetupWizardAutoShow = settings.suppressSetupWizardAutoShow,
legacySetupWizardStateMigrated = settings.legacySetupWizardStateMigrated,
showUnityCliLoopSecuritySetting = settings.showUnityCliLoopSecuritySetting,
showToolSettings = settings.showToolSettings,
installSkillsFlat = settings.installSkillsFlat
};
}
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,9 @@ public void SaveSettings(UnityCliLoopEditorSettingsData settings)
Directory.CreateDirectory(directory);
}

string json = JsonUtility.ToJson(settings, true);
UnityCliLoopEditorSettingsJsonData jsonData =
UnityCliLoopEditorSettingsJsonData.FromDomain(settings);
string json = JsonUtility.ToJson(jsonData, true);

// Security: Validate JSON content size
if (json.Length > UnityCliLoopConstants.MAX_SETTINGS_SIZE_BYTES)
Expand Down Expand Up @@ -192,7 +194,9 @@ private void LoadSettings()
throw new InvalidDataException("Settings file contains invalid JSON content");
}

_cachedSettings = JsonUtility.FromJson<UnityCliLoopEditorSettingsData>(json);
UnityCliLoopEditorSettingsJsonData loadedSettings =
JsonUtility.FromJson<UnityCliLoopEditorSettingsJsonData>(json);
_cachedSettings = loadedSettings.ToDomain();
}
else
{
Expand Down
Loading