Skip to content

Modernize DockerRegistryExplorer sample to .NET 8.0#31

Open
Jaben wants to merge 3 commits intodevelopfrom
feature/docker-registry-explorer
Open

Modernize DockerRegistryExplorer sample to .NET 8.0#31
Jaben wants to merge 3 commits intodevelopfrom
feature/docker-registry-explorer

Conversation

@Jaben
Copy link
Member

@Jaben Jaben commented Oct 6, 2025

Summary

Modernizes the DockerRegistryExplorer WPF sample application to .NET 8.0 and updates all dependencies to their latest versions.

Closes #25

Changes

Project Modernization

  • ✅ Converted from .NET Framework 4.8 to modern SDK-style project targeting net8.0-windows
  • ✅ Simplified project file from 186 lines to 26 lines
  • ✅ Enabled nullable reference types and implicit usings
  • ✅ Added GenerateAssemblyInfo=false to prevent duplicate attribute errors

Dependency Updates

Package Old Version New Version
Autofac 6.4.0 8.4.0
CommunityToolkit.Mvvm 8.0.0 8.4.0
Newtonsoft.Json 13.0.1 13.0.4
Serilog 2.12.0 4.3.0
Serilog.Sinks.Console 4.1.0 6.0.0
Serilog.Sinks.Debug 2.0.0 3.0.0

Code Modernization

Replaced Obsolete MvvmLight with CommunityToolkit.Mvvm

  • Migrated all ViewModels from GalaSoft.MvvmLight.ViewModelBase to CommunityToolkit.Mvvm.ComponentModel.ObservableObject
  • Replaced GalaSoft.MvvmLight.CommandWpf.RelayCommand with CommunityToolkit.Mvvm.Input.RelayCommand
  • Updated property change notifications: RaisePropertyChanged()OnPropertyChanged()
  • Removed deprecated DispatcherHelper.Initialize() call

Updated API Calls for New Docker.Registry.DotNet Conventions

  • Removed "Async" suffix from method names (e.g., GetManifestAsyncGetManifest)
  • Updated manifest operations to use ImageReference.Create() wrapper
  • Fixed ImageTag handling - extract .Value property when passing tags as strings
  • Updated fluent configuration API for authentication

Other Improvements

  • Fixed default Docker Hub endpoint: registry.hub.docker.comregistry-1.docker.io
  • Removed obsolete using statements
  • Cleaned up namespace organization

Build Status

✅ Builds successfully with 0 errors (only nullable reference warnings)

Testing

Tested with:

  • Local Docker registry (docker run -d -p 5000:5000 registry:2)
  • Docker Hub endpoint (catalog restricted, but repository operations work)

Test Plan

  • Build succeeds without errors
  • Application launches successfully
  • Can connect to local registry
  • Can list repositories
  • Can view tags for repositories
  • Can view manifest details
  • Authentication flow works

Migration Notes

⚠️ Cas.Common.WPF (v1.0.42) shows compatibility warnings as it's a .NET Framework package. It works but may need replacement with modern alternatives in the future.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added Refresh and Connect commands; tag actions for Get Manifest, View Manifest, Delete, and Copy Tag.
    • Enabled layer download with progress and completion messages.
  • Improvements

    • Modernized the sample app to a current Windows desktop runtime and updated dependencies.
    • Simplified sign-in flow and updated default registry endpoint to https://registry-1.docker.io.
    • Improved error handling and dialog interactions.
  • Bug Fixes

    • Fixed control namespace wiring so UI components load reliably.
  • Chores

    • Ignored local settings file (settings.local.json).

Jaben and others added 2 commits October 5, 2025 22:54
Closes #25

- Convert project from .NET Framework 4.8 to modern SDK-style targeting net8.0-windows
- Update all NuGet packages to latest versions:
  - Autofac: 6.4.0 → 8.4.0
  - CommunityToolkit.Mvvm: 8.0.0 → 8.4.0
  - Newtonsoft.Json: 13.0.1 → 13.0.4
  - Serilog: 2.12.0 → 4.3.0
  - Serilog.Sinks.Console: 4.1.0 → 6.0.0
  - Serilog.Sinks.Debug: 2.0.0 → 3.0.0
- Replace obsolete MvvmLight with CommunityToolkit.Mvvm:
  - Migrate all ViewModels from ViewModelBase to ObservableObject
  - Replace RelayCommand with CommunityToolkit.Mvvm.Input.RelayCommand
  - Update RaisePropertyChanged() to OnPropertyChanged()
  - Remove deprecated DispatcherHelper.Initialize()
- Update API calls for new Docker.Registry.DotNet conventions:
  - Remove "Async" suffix from method names
  - Use ImageReference.Create() for manifest operations
  - Extract ImageTag.Value when passing tags as strings
- Fix default Docker Hub endpoint to https://registry-1.docker.io
- Enable nullable reference types and implicit usings
- Disable auto-generated AssemblyInfo to prevent conflicts

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Oct 6, 2025

Walkthrough

Modernized the DockerRegistryExplorer sample to SDK-style .NET (net8.0-windows), migrated MVVM Light to CommunityToolkit.ObservableObject, updated DI/startup and global usings, adjusted namespaces/usings and XAML aliases, refined authentication/client flows, added Task extension, and reworked manifest/tag viewmodel command workflows.

Changes

Cohort / File(s) Summary
Project & global usings
samples/DockerRegistryExplorer/DockerRegistryExplorer.csproj, samples/DockerRegistryExplorer/GlobalUsings.cs, samples/DockerRegistryExplorer/Properties/AssemblyInfo.cs, .gitignore
Migrated to SDK-style net8.0-windows WPF project, updated PackageReferences and project settings, added global usings, removed legacy AssemblyInfo using, and added settings.local.json to .gitignore.
Startup & DI
samples/DockerRegistryExplorer/App.xaml.cs, samples/DockerRegistryExplorer/ContainerFactory.cs, samples/DockerRegistryExplorer/BuilderExtensions.cs
Removed DispatcherHelper.Initialize, preserved container build API, switched to file-scoped namespaces/modern formatting, and simplified DI registration extension.
Controls & XAML
samples/DockerRegistryExplorer/Controls/BusyIndicator.xaml, samples/DockerRegistryExplorer/Controls/BusyIndicator.xaml.cs, samples/DockerRegistryExplorer/View/ConnectView.xaml
Changed BusyIndicator x:Class/namespace to DockerRegistryExplorer.Controls, removed old local xmlns, and updated XAML alias usage.
View code-behind cleanup
samples/DockerRegistryExplorer/View/ConnectView.xaml.cs, samples/DockerRegistryExplorer/View/MainWindow.xaml.cs, samples/DockerRegistryExplorer/View/ManfiestDialogView.xaml.cs, samples/DockerRegistryExplorer/View/TextDialogView.xaml.cs
Removed unused System.Windows/System.Windows.Input usings; no public API changes.
Converters & utilities
samples/DockerRegistryExplorer/Converters/InverseBooleanConverter.cs, samples/DockerRegistryExplorer/TaskExtensions.cs
Switched to file-scoped namespace, adjusted usings and formatting; added public static void IgnoreAsync(this Task task) extension.
ViewModel base migration
samples/DockerRegistryExplorer/ViewModel/*.cs (e.g., AsyncExecutor.cs, CloseableViewModelBase.cs, DialogViewModelBase.cs, TextDialogViewModel.cs, MainViewModel.cs, RepositoriesViewModel.cs, RepositoryViewModel.cs, RegistryViewModel.cs, ManifestLayerViewModel.cs)
Replaced MVVM Light ViewModelBase with ObservableObject, updated property-change calls to OnPropertyChanged, refactored constructors/initializers and command exposure; kept public surfaces where noted but changed inheritance and internal wiring.
Manifest & tag workflows
samples/DockerRegistryExplorer/ViewModel/ManifestDialogViewModel.cs, .../ManifestLayerViewModel.cs, .../TagViewModel.cs
Reworked manifest download/view flows to use AsyncExecutor and explicit error handling, added SelectedLayer property, refactored TagViewModel into DI-driven command-oriented implementation (get/view manifest, delete, copy tag).
Connect/auth flow
samples/DockerRegistryExplorer/ViewModel/ConnectViewModel.cs
Changed default endpoint, simplified authentication to use configuration-based password OAuth when credentials provided, replaced PingAsync with Ping, and switched property-change notifications to OnPropertyChanged.
CI workflow
.github/workflows/publish.yml
Narrowed test run to a specific test project by invoking its csproj in the tests step.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant ConnectVM as ConnectViewModel
  participant Config as RegistryClientConfiguration
  participant Client as IRegistryClient

  User->>ConnectVM: Click Connect
  ConnectVM->>Config: Build configuration (endpoint)
  alt Username/Password provided
    ConnectVM->>Config: UsePasswordOAuthAuthentication(username, password)
  end
  ConnectVM->>Client: CreateClient()
  ConnectVM->>Client: Ping()
  Client-->>ConnectVM: Result (success/error)
  ConnectVM-->>User: Update UI / show message
Loading
sequenceDiagram
  autonumber
  actor User
  participant TagVM as TagViewModel
  participant Exec as AsyncExecutor
  participant Reg as IRegistryClient
  participant ViewSvc as IViewService

  User->>TagVM: ViewManifestCommand
  TagVM->>Exec: ExecuteAsync(GetManifestInternal)
  Exec->>Reg: GetManifest(repository, tag)
  Reg-->>Exec: Manifest or Error
  alt Error
    Exec-->>TagVM: ex
    TagVM-->>User: Show error
  else Manifest
    Exec-->>TagVM: manifest
    alt Text/JSON
      TagVM->>ViewSvc: Show TextDialog
    else Structured
      TagVM->>ViewSvc: Show ManifestDialog
    end
  end
Loading
sequenceDiagram
  autonumber
  actor User
  participant MVM as ManifestDialogViewModel
  participant Exec as AsyncExecutor
  participant Reg as IRegistryClient
  participant FileDlg as IFileDialogService
  participant FS as FileSystem

  User->>MVM: DownloadCommand
  MVM->>Exec: ExecuteAsync(GetBlob)
  Exec->>Reg: GetBlob(digest)
  Reg-->>Exec: Stream or Error
  alt Error
    Exec-->>MVM: ex
    MVM-->>User: Show error
  else Success
    Exec-->>MVM: blobStream
    MVM->>FileDlg: ShowSaveFileDialog()
    FileDlg-->>MVM: Path or cancel
    alt Path chosen
      MVM->>FS: Create file stream
      MVM->>FS: Copy blob -> file
      MVM-->>User: "Layer saved"
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Poem

I thump on net-eight, tidy and spry,
I swap old tails for Toolkit's eye.
I hop through DI, commands in tow,
Manifest crumbs in a neat little row.
— A cheery CodeRabbit 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Out of Scope Changes Check ⚠️ Warning The pull request includes an update to .gitignore to ignore settings.local.json and modifies the CI workflow to target a specific test project, neither of which relates directly to modernizing the DockerRegistryExplorer sample as described in issue #25. Please remove or relocate the .gitignore and CI workflow changes into a separate PR or clearly document how they support the sample modernization objectives.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title “Modernize DockerRegistryExplorer sample to .NET 8.0” clearly and concisely conveys the primary purpose of the pull request, namely updating the sample project to target .NET 8.0, making it immediately understandable to reviewers.
Linked Issues Check ✅ Passed The changes fully satisfy issue #25 by migrating the WPF sample to an SDK‐style .NET 8.0 project, updating all dependencies to their latest versions, replacing MvvmLight with CommunityToolkit.Mvvm, adjusting API calls to the Docker.Registry.DotNet library, and removing obsolete usings and patterns.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feature/docker-registry-explorer

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6374c10 and 6dcf2e4.

📒 Files selected for processing (1)
  • .github/workflows/publish.yml (1 hunks)
🔇 Additional comments (1)
.github/workflows/publish.yml (1)

35-35: Scoping tests to the library keeps Linux CI green
Pointing dotnet test at the library test project sidesteps building the new WPF sample (net8.0-windows) on Ubuntu runners, so the workflow stays portable. 👍


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

github-actions bot commented Oct 6, 2025

Test Results

12 tests   12 ✅  0s ⏱️
 1 suites   0 💤
 1 files     0 ❌

Results for commit 6dcf2e4.

♻️ This comment has been updated with latest results.

The DockerRegistryExplorer WPF sample project requires Windows to build.
Updated the test workflow to only run tests on the test project instead of
all projects in the solution to avoid building Windows-only samples on Linux.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions
Copy link

github-actions bot commented Oct 6, 2025

Code Coverage

Package Line Rate Branch Rate Complexity Health
Docker.Registry.DotNet 12% 19% 605
Summary 12% (155 / 1306) 19% (73 / 378) 605

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 5

🧹 Nitpick comments (4)
samples/DockerRegistryExplorer/ViewModel/ConnectViewModel.cs (1)

76-79: Consider validating for empty strings in addition to null.

The authentication condition checks for null but allows empty strings, which would trigger an authentication attempt with invalid credentials rather than connecting anonymously.

Apply this diff to improve the validation:

-if (!this.IsAnonymous && this.Username != null && this.Password != null)
+if (!this.IsAnonymous && !string.IsNullOrWhiteSpace(this.Username) && !string.IsNullOrWhiteSpace(this.Password))
     configuration.UsePasswordOAuthAuthentication(this.Username, this.Password);
samples/DockerRegistryExplorer/ViewModel/MainViewModel.cs (1)

13-20: Consider adding null check for lifetimeScope parameter.

Line 16 validates viewService with a null check, but line 15 assigns lifetimeScope without validation. For consistency and defensive programming, consider adding the same null guard.

Apply this diff to add consistent null checking:

 public MainViewModel(ILifetimeScope lifetimeScope, IViewService viewService)
 {
-    _lifetimeScope = lifetimeScope;
+    _lifetimeScope = lifetimeScope ?? throw new ArgumentNullException(nameof(lifetimeScope));
     _viewService = viewService ?? throw new ArgumentNullException(nameof(viewService));
samples/DockerRegistryExplorer/TaskExtensions.cs (1)

5-7: Consider logging unobserved exceptions.

The IgnoreAsync extension method implements fire-and-forget, but unobserved task exceptions can crash the app in some scenarios. Consider observing exceptions for logging even if you don't handle them.

Apply this diff to add basic exception observation:

-    public static void IgnoreAsync(this Task task)
+    public static async void IgnoreAsync(this Task task)
     {
+        try
+        {
+            await task.ConfigureAwait(false);
+        }
+        catch (Exception ex)
+        {
+            // Log but don't propagate - this is fire-and-forget
+            System.Diagnostics.Debug.WriteLine($"Unobserved exception in IgnoreAsync: {ex}");
+        }
     }
samples/DockerRegistryExplorer/BuilderExtensions.cs (1)

10-10: Typo in comment: "Registry" → "Register".

Apply this diff:

-        //Registry the view model itself
+        //Register the view model itself
         builder.RegisterType<TViewModel>();
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3e87bb2 and 6374c10.

📒 Files selected for processing (28)
  • .gitignore (1 hunks)
  • samples/DockerRegistryExplorer/App.xaml.cs (1 hunks)
  • samples/DockerRegistryExplorer/BuilderExtensions.cs (1 hunks)
  • samples/DockerRegistryExplorer/ContainerFactory.cs (1 hunks)
  • samples/DockerRegistryExplorer/Controls/BusyIndicator.xaml (1 hunks)
  • samples/DockerRegistryExplorer/Controls/BusyIndicator.xaml.cs (1 hunks)
  • samples/DockerRegistryExplorer/Converters/InverseBooleanConverter.cs (1 hunks)
  • samples/DockerRegistryExplorer/DockerRegistryExplorer.csproj (1 hunks)
  • samples/DockerRegistryExplorer/GlobalUsings.cs (1 hunks)
  • samples/DockerRegistryExplorer/Properties/AssemblyInfo.cs (0 hunks)
  • samples/DockerRegistryExplorer/TaskExtensions.cs (1 hunks)
  • samples/DockerRegistryExplorer/View/ConnectView.xaml (2 hunks)
  • samples/DockerRegistryExplorer/View/ConnectView.xaml.cs (0 hunks)
  • samples/DockerRegistryExplorer/View/MainWindow.xaml.cs (0 hunks)
  • samples/DockerRegistryExplorer/View/ManfiestDialogView.xaml.cs (0 hunks)
  • samples/DockerRegistryExplorer/View/TextDialogView.xaml.cs (0 hunks)
  • samples/DockerRegistryExplorer/ViewModel/AsyncExecutor.cs (2 hunks)
  • samples/DockerRegistryExplorer/ViewModel/CloseableViewModelBase.cs (1 hunks)
  • samples/DockerRegistryExplorer/ViewModel/ConnectViewModel.cs (6 hunks)
  • samples/DockerRegistryExplorer/ViewModel/DialogViewModelBase.cs (1 hunks)
  • samples/DockerRegistryExplorer/ViewModel/MainViewModel.cs (1 hunks)
  • samples/DockerRegistryExplorer/ViewModel/ManifestDialogViewModel.cs (1 hunks)
  • samples/DockerRegistryExplorer/ViewModel/ManifestLayerViewModel.cs (1 hunks)
  • samples/DockerRegistryExplorer/ViewModel/RegistryViewModel.cs (1 hunks)
  • samples/DockerRegistryExplorer/ViewModel/RepositoriesViewModel.cs (1 hunks)
  • samples/DockerRegistryExplorer/ViewModel/RepositoryViewModel.cs (1 hunks)
  • samples/DockerRegistryExplorer/ViewModel/TagViewModel.cs (1 hunks)
  • samples/DockerRegistryExplorer/ViewModel/TextDialogViewModel.cs (1 hunks)
💤 Files with no reviewable changes (5)
  • samples/DockerRegistryExplorer/Properties/AssemblyInfo.cs
  • samples/DockerRegistryExplorer/View/MainWindow.xaml.cs
  • samples/DockerRegistryExplorer/View/ConnectView.xaml.cs
  • samples/DockerRegistryExplorer/View/TextDialogView.xaml.cs
  • samples/DockerRegistryExplorer/View/ManfiestDialogView.xaml.cs
🧰 Additional context used
🧬 Code graph analysis (9)
samples/DockerRegistryExplorer/ViewModel/ManifestLayerViewModel.cs (1)
samples/DockerRegistryExplorer/ViewModel/AsyncExecutor.cs (1)
  • AsyncExecutor (10-56)
samples/DockerRegistryExplorer/ViewModel/RegistryViewModel.cs (3)
samples/DockerRegistryExplorer/ViewModel/RepositoriesViewModel.cs (3)
  • RepositoriesViewModel (9-114)
  • RepositoriesViewModel (21-37)
  • Refresh (106-108)
samples/DockerRegistryExplorer/ViewModel/MainViewModel.cs (1)
  • Refresh (52-55)
samples/DockerRegistryExplorer/ViewModel/RepositoryViewModel.cs (3)
  • Refresh (47-52)
  • RepositoryViewModel (5-74)
  • RepositoryViewModel (13-27)
samples/DockerRegistryExplorer/ViewModel/ManifestDialogViewModel.cs (4)
samples/DockerRegistryExplorer/ViewModel/CloseableViewModelBase.cs (1)
  • CloseableViewModelBase (6-23)
samples/DockerRegistryExplorer/ViewModel/TagViewModel.cs (2)
  • TagViewModel (6-168)
  • TagViewModel (18-41)
samples/DockerRegistryExplorer/ViewModel/ManifestLayerViewModel.cs (2)
  • ManifestLayerViewModel (5-30)
  • ManifestLayerViewModel (13-21)
samples/DockerRegistryExplorer/ViewModel/AsyncExecutor.cs (1)
  • AsyncExecutor (10-56)
samples/DockerRegistryExplorer/ContainerFactory.cs (1)
samples/DockerRegistryExplorer/BuilderExtensions.cs (1)
  • RegisterViewModel (7-15)
samples/DockerRegistryExplorer/ViewModel/TagViewModel.cs (4)
samples/DockerRegistryExplorer/ViewModel/RepositoryViewModel.cs (3)
  • RepositoryViewModel (5-74)
  • RepositoryViewModel (13-27)
  • Task (54-68)
samples/DockerRegistryExplorer/ViewModel/AsyncExecutor.cs (2)
  • AsyncExecutor (10-56)
  • Task (26-48)
samples/DockerRegistryExplorer/ViewModel/TextDialogViewModel.cs (2)
  • TextDialogViewModel (3-14)
  • TextDialogViewModel (5-9)
samples/DockerRegistryExplorer/ViewModel/ManifestDialogViewModel.cs (2)
  • ManifestDialogViewModel (8-112)
  • ManifestDialogViewModel (24-51)
samples/DockerRegistryExplorer/ViewModel/RepositoryViewModel.cs (5)
samples/DockerRegistryExplorer/ViewModel/TagViewModel.cs (3)
  • TagViewModel (6-168)
  • TagViewModel (18-41)
  • Task (86-101)
samples/DockerRegistryExplorer/ViewModel/RegistryViewModel.cs (3)
  • RegistryViewModel (3-28)
  • RegistryViewModel (5-16)
  • Refresh (24-27)
samples/DockerRegistryExplorer/ViewModel/RepositoriesViewModel.cs (3)
  • Refresh (106-108)
  • Task (62-76)
  • Task (85-104)
samples/DockerRegistryExplorer/ViewModel/AsyncExecutor.cs (2)
  • AsyncExecutor (10-56)
  • Task (26-48)
samples/DockerRegistryExplorer/TaskExtensions.cs (1)
  • IgnoreAsync (5-7)
samples/DockerRegistryExplorer/ViewModel/RepositoriesViewModel.cs (4)
samples/DockerRegistryExplorer/ViewModel/RegistryViewModel.cs (3)
  • RegistryViewModel (3-28)
  • RegistryViewModel (5-16)
  • Refresh (24-27)
samples/DockerRegistryExplorer/ViewModel/RepositoryViewModel.cs (5)
  • RepositoryViewModel (5-74)
  • RepositoryViewModel (13-27)
  • Task (54-68)
  • Refresh (47-52)
  • CanRefresh (70-73)
samples/DockerRegistryExplorer/ViewModel/AsyncExecutor.cs (2)
  • AsyncExecutor (10-56)
  • Task (26-48)
samples/DockerRegistryExplorer/TaskExtensions.cs (1)
  • IgnoreAsync (5-7)
samples/DockerRegistryExplorer/ViewModel/MainViewModel.cs (4)
samples/DockerRegistryExplorer/ViewModel/RegistryViewModel.cs (3)
  • RegistryViewModel (3-28)
  • RegistryViewModel (5-16)
  • Refresh (24-27)
samples/DockerRegistryExplorer/ViewModel/RepositoryViewModel.cs (1)
  • Refresh (47-52)
samples/DockerRegistryExplorer/ViewModel/AsyncExecutor.cs (1)
  • AsyncExecutor (10-56)
samples/DockerRegistryExplorer/ViewModel/ConnectViewModel.cs (1)
  • ConnectViewModel (8-87)
samples/DockerRegistryExplorer/ViewModel/DialogViewModelBase.cs (2)
samples/DockerRegistryExplorer/ViewModel/CloseableViewModelBase.cs (2)
  • CloseableViewModelBase (6-23)
  • RaiseCloseEventArgs (17-20)
samples/DockerRegistryExplorer/ViewModel/ConnectViewModel.cs (1)
  • Ok (67-72)
🪛 GitHub Actions: Build and Push to Nuget
samples/DockerRegistryExplorer/DockerRegistryExplorer.csproj

[error] 107-107: NETSDK1100: To build a project targeting Windows on this operating system, set the EnableWindowsTargeting property to true. (Triggered during: dotnet test --logger trx --collect:"XPlat Code Coverage")

🔇 Additional comments (18)
.gitignore (1)

289-289: Local settings ignore looks good.

Excluding settings.local.json keeps per-developer configuration and secrets out of source control.

samples/DockerRegistryExplorer/Controls/BusyIndicator.xaml.cs (1)

1-3: LGTM! Clean namespace migration.

The namespace update from DockerExplorer.Controls to DockerRegistryExplorer.Controls is consistent with the project modernization, and the reduction of using directives aligns with implicit usings enabled in .NET 8.

samples/DockerRegistryExplorer/View/ConnectView.xaml (1)

13-13: LGTM! Namespace alias correctly updated.

The addition of the xmlns:Controls namespace alias and the updated BusyIndicator reference align perfectly with the namespace changes in the control's code-behind file.

Also applies to: 111-111

samples/DockerRegistryExplorer/Controls/BusyIndicator.xaml (1)

1-1: LGTM! x:Class attribute correctly updated.

The x:Class attribute update matches the namespace change in the code-behind file (BusyIndicator.xaml.cs), ensuring proper XAML-to-code binding.

samples/DockerRegistryExplorer/ViewModel/ConnectViewModel.cs (3)

4-4: LGTM!

The import path update to Docker.Registry.DotNet.Application.Authentication and the corrected Docker Hub endpoint (https://registry-1.docker.io) align with the modernization objectives.

Also applies to: 10-10


22-60: LGTM!

The property notification updates from RaisePropertyChanged() to OnPropertyChanged() correctly implement the migration from MVVM Light to CommunityToolkit.Mvvm.


81-85: LGTM!

The updated API usage—CreateClient() and Ping() without the "Async" suffix—correctly follows the Docker.Registry.DotNet conventions.

samples/DockerRegistryExplorer/ViewModel/MainViewModel.cs (2)

5-11: LGTM! Clean migration to CommunityToolkit.Mvvm.

The change from ViewModelBase to ObservableObject aligns with the migration from MvvmLight to CommunityToolkit.Mvvm. The inline field initialization using target-typed new() expressions is modern and concise.


52-55: LGTM!

The refresh logic correctly iterates over all registries and invokes their Refresh() methods. This is clean and straightforward.

samples/DockerRegistryExplorer/DockerRegistryExplorer.csproj (2)

16-16: Cas.Common.WPF compatibility acknowledged.

As noted in the PR summary, Cas.Common.WPF v1.0.42 is a .NET Framework package and may emit compatibility warnings. Consider replacing it with a .NET-compatible alternative in a future update.


11-11: Assembly metadata provided manually
A manual AssemblyInfo.cs exists at samples/DockerRegistryExplorer/Properties/AssemblyInfo.cs, so disabling GenerateAssemblyInfo is safe.

samples/DockerRegistryExplorer/Converters/InverseBooleanConverter.cs (1)

1-31: LGTM! Modernization changes applied correctly.

The file has been successfully modernized with file-scoped namespace, explicit using for System.Globalization, and minor formatting improvements. No functional changes, and the implementation remains correct.

samples/DockerRegistryExplorer/App.xaml.cs (1)

10-20: LGTM! Correct migration from MvvmLight dispatcher initialization.

The removal of DispatcherHelper.Initialize() is appropriate for the migration to CommunityToolkit.Mvvm, which doesn't require manual dispatcher initialization. The container lifecycle management with using is correct for the modal dialog pattern.

samples/DockerRegistryExplorer/GlobalUsings.cs (1)

1-26: LGTM! Global usings well-chosen for WPF app.

The global using directives appropriately consolidate common namespaces for WPF, MVVM, DI, and the Docker Registry API. This aligns well with the ImplicitUsings=enable setting in the project file.

samples/DockerRegistryExplorer/ViewModel/AsyncExecutor.cs (2)

10-10: LGTM! Correct migration to ObservableObject.

The base class migration from ViewModelBase to ObservableObject is correct for the CommunityToolkit.Mvvm migration.


22-22: LGTM! Correct property notification method.

The change from RaisePropertyChanged() to OnPropertyChanged() is the correct equivalent in CommunityToolkit.Mvvm.

samples/DockerRegistryExplorer/BuilderExtensions.cs (1)

1-16: LGTM! DI registration logic is correct.

The file-scoped namespace modernization and DI registration pattern are appropriate for the migration.

samples/DockerRegistryExplorer/ViewModel/CloseableViewModelBase.cs (1)

6-6: LGTM! Correct base class migration.

The change from ViewModelBase to ObservableObject aligns with the CommunityToolkit.Mvvm migration while preserving the ICloseableViewModel interface contract.

Comment on lines +1 to +12
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>WinExe</OutputType>
<TargetFramework>net8.0-windows</TargetFramework>
<UseWPF>true</UseWPF>
<Nullable>enable</Nullable>
<ImplicitUsings>enable</ImplicitUsings>
<LangVersion>latest</LangVersion>
<AssemblyName>DockerRegistryExplorer</AssemblyName>
<RootNamespace>DockerRegistryExplorer</RootNamespace>
<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
</PropertyGroup>
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Pipeline failure: Add EnableWindowsTargeting for non-Windows CI builds.

The pipeline fails with NETSDK1100 because Windows-targeted projects require EnableWindowsTargeting=true on non-Windows build agents. While the app itself requires Windows, enabling this property allows the build system to validate the project structure on Linux/macOS CI runners.

Apply this diff to fix the pipeline failure:

 <Project Sdk="Microsoft.NET.Sdk">
 	<PropertyGroup>
 		<OutputType>WinExe</OutputType>
 		<TargetFramework>net8.0-windows</TargetFramework>
 		<UseWPF>true</UseWPF>
+		<EnableWindowsTargeting>true</EnableWindowsTargeting>
 		<Nullable>enable</Nullable>
 		<ImplicitUsings>enable</ImplicitUsings>
 		<LangVersion>latest</LangVersion>
 		<AssemblyName>DockerRegistryExplorer</AssemblyName>
 		<RootNamespace>DockerRegistryExplorer</RootNamespace>
 		<GenerateAssemblyInfo>false</GenerateAssemblyInfo>
 	</PropertyGroup>
🤖 Prompt for AI Agents
In samples/DockerRegistryExplorer/DockerRegistryExplorer.csproj around lines 1
to 12, the project targets Windows and fails CI on non-Windows agents; add
<EnableWindowsTargeting>true</EnableWindowsTargeting> inside the existing
PropertyGroup so the SDK will allow Windows-targeted validation on Linux/macOS
runners; update the PropertyGroup to include this property (no other changes).

Comment on lines +30 to +50
private void Connect()
{
var viewModel = _lifetimeScope.Resolve<ConnectViewModel>();

private void Refresh()
if (_viewService.ShowDialog(viewModel) ?? false)
{
foreach (var registry in Registries)
var registryClient = viewModel.RegistryClient;

var childScope = _lifetimeScope.BeginLifetimeScope(builder =>
{
registry.Refresh();
}
builder.RegisterInstance(registryClient);
});

var registry = childScope.Resolve<RegistryViewModel>
(
new NamedParameter("url", viewModel.Endpoint)
);

_registries.Add(registry);
}
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Critical: Child lifetime scope is never disposed.

Lines 38-41 create a child lifetime scope but the childScope variable is lost after the method returns without being disposed. This causes a resource leak because:

  1. Autofac lifetime scopes implement IDisposable and should be disposed when no longer needed
  2. Services resolved from the scope (like RegistryViewModel on lines 43-46) remain tied to that scope
  3. Accumulating undisposed scopes can lead to memory leaks

The scope should be stored alongside each RegistryViewModel and disposed when the registry is removed from the collection or when the application shuts down.

Consider one of these approaches:

Approach 1: Store the scope with the registry

Create a wrapper class:

private class RegistryEntry
{
    public RegistryViewModel ViewModel { get; init; }
    public ILifetimeScope Scope { get; init; }
}

Then modify the code:

-private readonly ObservableCollection<RegistryViewModel> _registries = new();
+private readonly ObservableCollection<RegistryEntry> _registries = new();

-public IEnumerable<RegistryViewModel> Registries => _registries;
+public IEnumerable<RegistryViewModel> Registries => _registries.Select(e => e.ViewModel);

 var childScope = _lifetimeScope.BeginLifetimeScope(builder =>
 {
     builder.RegisterInstance(registryClient);
 });

 var registry = childScope.Resolve<RegistryViewModel>
 (
     new NamedParameter("url", viewModel.Endpoint)
 );

-_registries.Add(registry);
+_registries.Add(new RegistryEntry { ViewModel = registry, Scope = childScope });

And implement proper cleanup when registries are removed.

Approach 2: Use a different scoping strategy

If the registries are meant to live for the application lifetime, consider registering them as singletons in the root container instead of using child scopes.

🤖 Prompt for AI Agents
In samples/DockerRegistryExplorer/ViewModel/MainViewModel.cs around lines 30 to
50, the child ILifetimeScope created for each registry is never disposed causing
resource leaks; fix by associating the scope with the resolved RegistryViewModel
(e.g., create a RegistryEntry wrapper that holds RegistryViewModel and
ILifetimeScope, add RegistryEntry instances to the _registries collection
instead of raw view models), update any UI/collection code to use
RegistryEntry.ViewModel where needed, and ensure you dispose the associated
ILifetimeScope when an entry is removed or when the application shuts down (or
alternatively change registration to register registries as singletons in the
root scope if they should live for the app lifetime).

Comment on lines +93 to 105
using (var stream = response.Stream)
{
using (var stream = response.Stream)
{
var result = this._fileDialogService.ShowSaveFileDialog();
var result = _fileDialogService.ShowSaveFileDialog();

if (result != null)
if (result != null)
using (var targetStream = File.Create(result.FileName))
{
using (var targetStream = File.Create(result.FileName))
{
await stream.CopyToAsync(targetStream);
}
await stream.CopyToAsync(targetStream);
}
}

this._messageBoxService.Show("Layer saved", "Complete");
}
}

private bool CanDownload()
{
return this.SelectedLayer != null;
_messageBoxService.Show("Layer saved", "Complete");
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Show success message only when a file is saved

If the user cancels the save dialog (result == null), we still display “Layer saved”, which is incorrect. Wrap the message inside the “file saved” branch (or return early) so we only signal success when the copy actually happens.

Apply this diff:

-            using (var stream = response.Stream)
-            {
-                var result = _fileDialogService.ShowSaveFileDialog();
-
-                if (result != null)
-                    using (var targetStream = File.Create(result.FileName))
-                    {
-                        await stream.CopyToAsync(targetStream);
-                    }
-            }
-
-            _messageBoxService.Show("Layer saved", "Complete");
+            using (var stream = response.Stream)
+            {
+                var result = _fileDialogService.ShowSaveFileDialog();
+
+                if (result != null)
+                {
+                    using (var targetStream = File.Create(result.FileName))
+                    {
+                        await stream.CopyToAsync(targetStream);
+                    }
+
+                    _messageBoxService.Show("Layer saved", "Complete");
+                }
+            }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
using (var stream = response.Stream)
{
using (var stream = response.Stream)
{
var result = this._fileDialogService.ShowSaveFileDialog();
var result = _fileDialogService.ShowSaveFileDialog();
if (result != null)
if (result != null)
using (var targetStream = File.Create(result.FileName))
{
using (var targetStream = File.Create(result.FileName))
{
await stream.CopyToAsync(targetStream);
}
await stream.CopyToAsync(targetStream);
}
}
this._messageBoxService.Show("Layer saved", "Complete");
}
}
private bool CanDownload()
{
return this.SelectedLayer != null;
_messageBoxService.Show("Layer saved", "Complete");
}
using (var stream = response.Stream)
{
var result = _fileDialogService.ShowSaveFileDialog();
if (result != null)
{
using (var targetStream = File.Create(result.FileName))
{
await stream.CopyToAsync(targetStream);
}
_messageBoxService.Show("Layer saved", "Complete");
}
}
🤖 Prompt for AI Agents
In samples/DockerRegistryExplorer/ViewModel/ManifestDialogViewModel.cs around
lines 93 to 105, the success message "_messageBoxService.Show(\"Layer saved\",
\"Complete\");" is shown even when the save dialog was cancelled (result ==
null); modify the flow so the message is only shown after a file was actually
created and the copy finished—either move the _messageBoxService.Show call
inside the if (result != null) block after the await
stream.CopyToAsync(targetStream) or return early when result == null so the
success message is not executed.

Comment on lines +24 to 27
public void Refresh()
{
foreach (var child in Children.OfType<RepositoryViewModel>()) child.Refresh();
}
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Fix Refresh child type filtering

Children is populated with RepositoriesViewModel instances, so filtering for RepositoryViewModel yields nothing and the refresh command silently does nothing. Iterate over the correct type so the underlying repositories actually refresh.

Apply this diff:

-        foreach (var child in Children.OfType<RepositoryViewModel>()) child.Refresh();
+        foreach (var child in Children.OfType<RepositoriesViewModel>()) child.Refresh();
🤖 Prompt for AI Agents
In samples/DockerRegistryExplorer/ViewModel/RegistryViewModel.cs around lines 24
to 27, the Refresh method filters Children for RepositoryViewModel but Children
actually contains RepositoriesViewModel instances, so nothing is refreshed;
change the iteration to Children.OfType<RepositoriesViewModel>() (or the correct
concrete type used when populating Children) and call child.Refresh() for each
so the underlying repositories are properly refreshed.

Comment on lines +64 to +75
var catalog =
await _registryClient.Catalog.GetCatalog(new CatalogParameters());

var repositories = catalog.Repositories.Select(
r => this._lifetimeScope.Resolve<RepositoryViewModel>(
new NamedParameter("name", r),
new TypedParameter(typeof(RegistryViewModel), this._parent)))
.OrderBy(e => e.Name)
.ToList();
var repositories = catalog.Repositories.Select(r => _lifetimeScope.Resolve<RepositoryViewModel>(
new NamedParameter("name", r),
new TypedParameter(typeof(RegistryViewModel), _parent)))
.OrderBy(e => e.Name)
.ToList();

this.Repositories = new ObservableCollection<RepositoryViewModel>(repositories);
Repositories = new ObservableCollection<RepositoryViewModel>(repositories);

Log.Debug("Done Getting Catalog {@Repositories}", repositories);
}

private void LoadRepository()
{
if (this.Executor.IsBusy) return;
Log.Debug("Done Getting Catalog {@Repositories}", repositories);
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Handle null catalog.Repositories before projecting.
Some registries (or transient responses) return a catalog payload with repositories omitted/null. This code will currently throw a NullReferenceException, breaking the refresh flow. Guard the sequence before projecting so the view model can continue with an empty list.

-        var repositories = catalog.Repositories.Select(r => _lifetimeScope.Resolve<RepositoryViewModel>(
-                new NamedParameter("name", r),
-                new TypedParameter(typeof(RegistryViewModel), _parent)))
-            .OrderBy(e => e.Name)
-            .ToList();
+        var repositoryNames = catalog.Repositories ?? Enumerable.Empty<string>();
+
+        var repositories = repositoryNames
+            .Select(r => _lifetimeScope.Resolve<RepositoryViewModel>(
+                new NamedParameter("name", r),
+                new TypedParameter(typeof(RegistryViewModel), _parent)))
+            .OrderBy(e => e.Name)
+            .ToList();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
var catalog =
await _registryClient.Catalog.GetCatalog(new CatalogParameters());
var repositories = catalog.Repositories.Select(
r => this._lifetimeScope.Resolve<RepositoryViewModel>(
new NamedParameter("name", r),
new TypedParameter(typeof(RegistryViewModel), this._parent)))
.OrderBy(e => e.Name)
.ToList();
var repositories = catalog.Repositories.Select(r => _lifetimeScope.Resolve<RepositoryViewModel>(
new NamedParameter("name", r),
new TypedParameter(typeof(RegistryViewModel), _parent)))
.OrderBy(e => e.Name)
.ToList();
this.Repositories = new ObservableCollection<RepositoryViewModel>(repositories);
Repositories = new ObservableCollection<RepositoryViewModel>(repositories);
Log.Debug("Done Getting Catalog {@Repositories}", repositories);
}
private void LoadRepository()
{
if (this.Executor.IsBusy) return;
Log.Debug("Done Getting Catalog {@Repositories}", repositories);
var catalog =
await _registryClient.Catalog.GetCatalog(new CatalogParameters());
var repositoryNames = catalog.Repositories ?? Enumerable.Empty<string>();
var repositories = repositoryNames
.Select(r => _lifetimeScope.Resolve<RepositoryViewModel>(
new NamedParameter("name", r),
new TypedParameter(typeof(RegistryViewModel), _parent)))
.OrderBy(e => e.Name)
.ToList();
Repositories = new ObservableCollection<RepositoryViewModel>(repositories);
Log.Debug("Done Getting Catalog {@Repositories}", repositories);
🤖 Prompt for AI Agents
In samples/DockerRegistryExplorer/ViewModel/RepositoriesViewModel.cs around
lines 64 to 75, the code assumes catalog.Repositories is non-null which can
throw a NullReferenceException; fix by guarding the projection with a null check
or null-coalescing fallback (e.g., use catalog.Repositories ??
Enumerable.Empty<string>()), then continue to resolve RepositoryViewModel,
OrderBy, ToList and assign an empty ObservableCollection when there are no
entries; ensure the Log.Debug call receives the resulting repositories list
(which will be an empty list instead of null).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TODO: Update the Registery Explorer Sample

1 participant