[ADR-178] ICloudAsset abstraction, JsonCloudAsset<T>, and stub file downloader - #148
Conversation
…ownloader Introduces the ICloudAsset/ICloudAsset<T> abstraction with BasicCloudAsset<T> and JsonCloudAsset<T>, a FileCloudAssetDownloader that reads from a stub JSON file, and dev/layout.json for both WPF and Headless hosts. CloudAssetOrchestrator now loops over IEnumerable<ICloudAsset>, downloading and parsing each asset via the injected ICloudAssetDownloader rather than constructing a hardcoded CompiledLayout in C#. This exercises the same download→parse→store path the real HTTP-backed orchestrator will use. AddScopedCloudAsset<T> now accepts an ICloudAsset<T> instance directly and co-locates the ICloudAsset singleton registration with the scoped DI accessor. CloudAssetStoreExtensions (SetLayout/GetLayout helpers) is deleted as unused. The "Info" TiVo button in layout.json is absent from the old hardcoded stub, so E2E assertions on it prove the file is being loaded and parsed correctly. https://claude.ai/code/session_01VHZuzd1UHqNKDxtkLZu7pz
There was a problem hiding this comment.
Pull request overview
Introduces a first-pass “cloud asset” abstraction centered around ICloudAsset, adds a JSON-backed asset implementation, and switches startup initialization to load a compiled layout from a stub JSON file (rather than a hardcoded in-memory layout).
Changes:
- Add
BasicCloudAsset<T>andJsonCloudAsset<T>for JSON deserialization of cloud assets. - Add
FileCloudAssetDownloaderand reworkCloudAssetOrchestratorto download/parse/store assets at startup. - Add stub compiled-layout JSON files + Development config to point
CloudSettings:StubFilePathat the stub, plus new/updated E2E + unit tests.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| test/AdaptiveRemote.EndToEndTests.Host.Wpf/Features/Shared/LayoutButtons.feature | Adds an assertion for the new Info button in the expected layout. |
| test/AdaptiveRemote.EndToEndTests.Host.Headless/Features/Layout/StubLayout.feature | New headless E2E scenario verifying stub JSON layout loads and enables Info. |
| test/AdaptiveRemote.App.Tests/Services/CloudAssets/JsonCloudAssetTests.cs | Unit test for JSON deserialization of CompiledLayout. |
| test/AdaptiveRemote.App.Tests/Services/CloudAssets/FileCloudAssetDownloaderTests.cs | Unit tests for stub-file downloader behavior. |
| src/AdaptiveRemote/dev/layout.json | Adds a stub compiled layout JSON file (WPF app). |
| src/AdaptiveRemote/appsettings.Development.json | Adds CloudSettings:StubFilePath pointing at the stub layout. |
| src/AdaptiveRemote/AdaptiveRemote.csproj | Copies stub dev/layout.json to output. |
| src/AdaptiveRemote.Headless/dev/layout.json | Adds a stub compiled layout JSON file (headless host). |
| src/AdaptiveRemote.Headless/appsettings.Development.json | Adds Development config for stub file path (headless). |
| src/AdaptiveRemote.Headless/AdaptiveRemote.Headless.csproj | Copies stub config + dev/layout.json to output. |
| src/AdaptiveRemote.App/Services/CloudAssets/JsonCloudAsset.cs | New JSON cloud asset implementation using JsonSerializerContext. |
| src/AdaptiveRemote.App/Services/CloudAssets/FileCloudAssetDownloader.cs | New downloader that reads the “active” asset from a local stub JSON file. |
| src/AdaptiveRemote.App/Services/CloudAssets/CloudSettings.cs | Adds StubFilePath option. |
| src/AdaptiveRemote.App/Services/CloudAssets/CloudAssetStoreExtensions.cs | Removes layout-specific store extension methods. |
| src/AdaptiveRemote.App/Services/CloudAssets/CloudAssetOrchestrator.cs | Replaces hardcoded stub layout with download/parse/store orchestration for registered assets. |
| src/AdaptiveRemote.App/Services/CloudAssets/BasicCloudAsset.cs | New base class for ICloudAsset<T> implementations. |
| src/AdaptiveRemote.App/Configuration/HostBuilderExtensions.cs | Registers the layout asset as a JsonCloudAsset<CompiledLayout>. |
| src/AdaptiveRemote.App/Configuration/CloudAssetServiceExtensions.cs | Registers orchestrator + stub downloader and updates AddScopedCloudAsset registration shape. |
| "CloudSettings": { | ||
| "StubFilePath": "dev/layout.json" | ||
| } |
There was a problem hiding this comment.
This setting is only present in appsettings.Development.json, but the WPF/console hosts use Host.CreateDefaultBuilder (driven by DOTNET_ENVIRONMENT). The WPF/console E2E host settings shown in test hooks don’t set DOTNET_ENVIRONMENT=Development, so this CloudSettings:StubFilePath will likely be ignored and the app will fail to load assets at startup. Consider either adding CloudSettings to appsettings.json as a safe default for now, or ensuring the test hosts set the correct environment variable.
| internal static IServiceCollection AddCloudAssetServices(this IServiceCollection services) | ||
| => services | ||
| .AddSingleton<ICloudAssetStore, CloudAssetStore>() | ||
| .AddSingleton<ICloudAssetDownloader, FileCloudAssetDownloader>() | ||
| .AddSingleton<CloudAssetOrchestrator>() | ||
| .AddSingleton<IPreScopeInitializer>(sp => sp.GetRequiredService<CloudAssetOrchestrator>()) | ||
| .AddHostedService(sp => sp.GetRequiredService<CloudAssetOrchestrator>()); |
There was a problem hiding this comment.
AddCloudAssetServices registers FileCloudAssetDownloader as the only ICloudAssetDownloader, but CloudSettings.StubFilePath defaults to an empty string and src/AdaptiveRemote/appsettings.json has no CloudSettings section. In non-Development environments this makes GetActiveAsync return null and CloudAssetOrchestrator throws, preventing the app from starting. Consider making downloader selection conditional (stub downloader only when StubFilePath is configured) and/or providing a non-empty default/fallback downloader so Production startup isn't broken.
| public int IdleCooldownSeconds { get; set; } = 30; | ||
| public int SseMaxConsecutiveFailures { get; set; } = 10; | ||
| public string CachePath { get; set; } = @"%LocalAppData%\AdaptiveRemote\CloudAssets"; | ||
| public string StubFilePath { get; set; } = ""; |
There was a problem hiding this comment.
StubFilePath defaults to an empty string, but the current DI setup always uses FileCloudAssetDownloader and CloudAssetOrchestrator treats a missing file as fatal. Unless every runtime environment provides CloudSettings:StubFilePath, this will cause startup failure. Either provide a sensible default (e.g., the bundled dev/layout.json) or change the downloader/orchestrator behavior so an unset stub path doesn't break startup.
| publicstringStubFilePath{get;set;}=""; | |
| publicstringStubFilePath{get;set;}=global::System.IO.Path.Combine(global::System.AppContext.BaseDirectory,"dev","layout.json"); |
| string path = Environment.ExpandEnvironmentVariables(_settings.StubFilePath); | ||
| if (!_fileSystem.FileExists(path)) | ||
| { | ||
| return Task.FromResult<Stream?>(null); | ||
| } | ||
| return Task.FromResult<Stream?>(_fileSystem.OpenRead(path)); | ||
| } | ||
| public Task<Stream?> GetByIdAsync(string resourcePath, Guid id, CancellationToken ct) | ||
| => Task.FromResult<Stream?>(null); |
There was a problem hiding this comment.
GetActiveAsync uses StubFilePath verbatim (after env var expansion). With a relative path like dev/layout.json, the lookup depends on the process working directory, which can differ between dotnet run, test hosts, and deployed installs. Consider resolving relative paths against a stable base (e.g., AppContext.BaseDirectory or content root) and failing fast with a clear exception when StubFilePath is empty/whitespace.
| stringpath=Environment.ExpandEnvironmentVariables(_settings.StubFilePath); | |
| if(!_fileSystem.FileExists(path)) | |
| { | |
| returnTask.FromResult<Stream?>(null); | |
| } | |
| returnTask.FromResult<Stream?>(_fileSystem.OpenRead(path)); | |
| } | |
| publicTask<Stream?>GetByIdAsync(stringresourcePath,Guidid,CancellationTokenct) | |
| =>Task.FromResult<Stream?>(null); | |
| stringpath=GetResolvedStubFilePath(); | |
| if(!_fileSystem.FileExists(path)) | |
| { | |
| returnTask.FromResult<Stream?>(null); | |
| } | |
| returnTask.FromResult<Stream?>(_fileSystem.OpenRead(path)); | |
| } | |
| publicTask<Stream?>GetByIdAsync(stringresourcePath,Guidid,CancellationTokenct) | |
| =>Task.FromResult<Stream?>(null); | |
| privatestringGetResolvedStubFilePath() | |
| { | |
| if(string.IsNullOrWhiteSpace(_settings.StubFilePath)) | |
| { | |
| thrownewInvalidOperationException("CloudSettings.StubFilePath must be configured with a non-empty file path."); | |
| } | |
| stringexpandedPath=Environment.ExpandEnvironmentVariables(_settings.StubFilePath); | |
| if(string.IsNullOrWhiteSpace(expandedPath)) | |
| { | |
| thrownewInvalidOperationException("CloudSettings.StubFilePath resolved to an empty file path after environment variable expansion."); | |
| } | |
| returnPath.IsPathRooted(expandedPath) | |
| ?Path.GetFullPath(expandedPath) | |
| :Path.GetFullPath(expandedPath,AppContext.BaseDirectory); | |
| } |
| foreach (ICloudAsset asset in _assets) | ||
| { | ||
| Stream stream = await _downloader.GetActiveAsync(asset.ResourcePath, stoppingToken) | ||
| ?? throw new InvalidOperationException($"Failed to download asset '{asset.Name}'."); |
There was a problem hiding this comment.
The exception thrown when a download returns null ("Failed to download asset '{asset.Name}'.") doesn’t include enough context to diagnose configuration problems (e.g., which resourcePath was requested, whether this is the stub downloader, or what stub path was used). Consider including at least asset.ResourcePath and relevant downloader/config details in the error so failures are actionable from logs.
| ??thrownewInvalidOperationException($"Failed to download asset '{asset.Name}'."); | |
| ??thrownewInvalidOperationException( | |
| $"Failed to download asset '{asset.Name}' for resource path '{asset.ResourcePath}'. Downloader: '{_downloader.GetType().FullName}'."); |
| protected override async Task ExecuteAsync(CancellationToken stoppingToken) | ||
| { | ||
| CompiledLayout layout = new( | ||
| Id: Guid.Empty, | ||
| RawLayoutId: Guid.Empty, | ||
| UserId: "stub", | ||
| IsActive: true, | ||
| Version: 1, | ||
| Elements: | ||
| [ | ||
| new LayoutGroupDefinitionDto("DPAD", | ||
| [ | ||
| new CommandDefinitionDto(CommandType.TiVo, "Up", "Up", null, "Sent Up", "Down", "Up"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Down", "Down", null, "Sent Down", "Up", "Down"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Left", "Left", null, "Sent Left", "Right", "Left"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Right", "Right", null, "Sent Right", "Left", "Right"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Select", "Select", null, "Sent Select", null, "Select"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Back", "Back", null, "Sent Back", null, "Back"), | ||
| new CommandDefinitionDto(CommandType.IR, "Power", "Power", null, "Sent Power", "Power", "Power"), | ||
| new CommandDefinitionDto(CommandType.IR, "PowerOn", "PowerOn", null, "Sent PowerOn", "PowerOff", "PowerOn"), | ||
| new CommandDefinitionDto(CommandType.IR, "PowerOff", "PowerOff", null, "Sent PowerOff", "PowerOn", "PowerOff"), | ||
| ]), | ||
| new LayoutGroupDefinitionDto("WELL", | ||
| [ | ||
| new CommandDefinitionDto(CommandType.TiVo, "TiVo", "TiVo", null, "Sent TiVo", null, "TiVo"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Netflix", "Netflix", null, "Sent Netflix", null, "Netflix"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Guide", "Guide", null, "Sent Guide", null, "Guide"), | ||
| ]), | ||
| new LayoutGroupDefinitionDto("PLAYBACK", | ||
| [ | ||
| new CommandDefinitionDto(CommandType.TiVo, "Play", "Play", null, "Sent Play", "Pause", "Play"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Pause", "Pause", null, "Sent Pause", "Play", "Pause"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Record", "Record", null, "Sent Record", null, "Record"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Skip", "Skip", null, "Sent Skip", "Replay", "Skip"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "Replay", "Replay", null, "Sent Replay", "Skip", "Replay"), | ||
| ]), | ||
| new LayoutGroupDefinitionDto("CHANNELANDVOLUME", | ||
| [ | ||
| new CommandDefinitionDto(CommandType.TiVo, "ChannelUp", "Up", null, "Sent Channel Up", "ChannelDown", "ChannelUp"), | ||
| new CommandDefinitionDto(CommandType.TiVo, "ChannelDown", "Down", null, "Sent Channel Down", "ChannelUp", "ChannelDown"), | ||
| new CommandDefinitionDto(CommandType.IR, "VolumeUp", "Up", null, "Sent Volume Up", "VolumeDown", "VolumeUp"), | ||
| new CommandDefinitionDto(CommandType.IR, "VolumeDown", "Down", null, "Sent Volume Down", "VolumeUp", "VolumeDown"), | ||
| new CommandDefinitionDto(CommandType.IR, "Mute", "Mute", null, "Sent Mute", "Mute", "Mute"), | ||
| ]), | ||
| ], | ||
| CssDefinitions: "", | ||
| CompiledAt: DateTimeOffset.UtcNow); | ||
| _store.SetLayout(layout); | ||
| _initCompleted.SetResult(); | ||
| return Task.CompletedTask; | ||
| try | ||
| { | ||
| foreach (ICloudAsset asset in _assets) | ||
| { | ||
| Stream stream = await _downloader.GetActiveAsync(asset.ResourcePath, stoppingToken) | ||
| ?? throw new InvalidOperationException($"Failed to download asset '{asset.Name}'."); | ||
| await using (stream) | ||
| { | ||
| object value = await asset.ParseAsync(stream, stoppingToken); | ||
| _store.Set(asset.Name, value); | ||
| } | ||
| } | ||
| _initCompleted.SetResult(); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _initCompleted.TrySetException(ex); | ||
| throw; | ||
| } |
There was a problem hiding this comment.
CloudAssetOrchestrator.ExecuteAsync now contains the core initialization logic (download → parse → store) and is a startup gate via IPreScopeInitializer, but there are no unit tests covering success and failure paths (e.g., null stream, parse failure, cancellation). Adding tests would help prevent regressions since failures here block the app from reaching Ready phase.
- Rename ParseAsync → DeserializeAsync on ICloudAsset/BasicCloudAsset/JsonCloudAsset
(ParseAsync was misleading; DeserializeAsync is more precise for all asset types)
- Add logging to CloudAssetOrchestrator: logs each asset download (1700) and
initialization failure (1701); verified in new CloudAssetOrchestratorTests
- Consolidate CloudSettings with BackendSettings: remove duplicate backend
credential fields (BackendBaseUrl, CognitoTokenEndpointUrl, ClientId, ClientSecret)
that duplicated BackendSettings; change StubFilePath default to "dev/layout.json"
so the app works without requiring ASPNETCORE_ENVIRONMENT=Development
- Add [JsonConverter(typeof(JsonStringEnumConverter<CommandType>))] to CommandType
so enum values serialize as names ("TiVo", "IR") rather than integers; update
both layout.json files accordingly
- Rewrite JsonCloudAssetTests to use a test-specific TestAsset record rather than
CompiledLayout, making the tests robust to contract changes
- Add CloudAssetOrchestratorTests covering success, null-stream, and parse-failure
paths with log verification
- Delete StubLayout.feature (headless-specific); the LayoutButtons.feature shared
scenario already asserts the Info button present only in layout.json
https://claude.ai/code/session_01VHZuzd1UHqNKDxtkLZu7pz0bc1251
into
feature/ADR-162-client-side-layout-updatesUh oh!
There was an error while loading. Please reload this page.
Introduce the asset abstraction and replace in-code CompiledLayout construction with JSON deserialization from a file, so the stub orchestrator exercises the same parse path as the real orchestrator will.