DockerService, ManifestService, ImageDigestCache, and RegistryServiceClient classes are all too tightly coupled

[lbussell]

At a high level, DockerService depends on the ManifestService. However, DockerService only relies on the ManifestService for methods that have the exact same signature and behavior as ManifestService commands (or its extension methods).

docker-tools/src/Microsoft.DotNet.ImageBuilder/src/DockerService.cs

Lines 59 to 60 in f2d23ef

	public Task<IEnumerable<string>> GetImageManifestLayersAsync(string image, IRegistryCredentialsHost credsHost, bool isDryRun) =>
	_manifestService.GetImageLayersAsync(image, credsHost, isDryRun);

The DockerServiceCache is also tightly coupled with the ImageDigestCache class because of the same reason. ImageDigestCache simply makes and caches calls to the ManifestService class (through DockerService).

docker-tools/src/Microsoft.DotNet.ImageBuilder/src/DockerServiceCache.cs

Lines 55 to 56 in f2d23ef

	public Task<string?> GetImageDigestAsync(string image, IRegistryCredentialsHost credsHost, bool isDryRun) =>
	_imageDigestCache.GetImageDigestAsync(image, credsHost, isDryRun);

docker-tools/src/Microsoft.DotNet.ImageBuilder/src/ImageDigestCache.cs

Lines 37 to 43 in f2d23ef

	public Task<string?> GetImageDigestAsync(string tag, IRegistryCredentialsHost credsHost, bool isDryRun) =>
	LockHelper.DoubleCheckedLockLookupAsync(_digestCacheLock, _digestCache, tag,
	() => _dockerService.GetImageDigestAsync(tag, credsHost, isDryRun),
	// Don't allow null digests to be cached. A locally built image won't have a digest until
	// it is pushed so if its digest is retrieved before pushing, we don't want that
	// null to be cached.
	val => !string.IsNullOrEmpty(val));

We should remove DockerService's dependency on the ManifestService class, since there are commands that use the DockerService that don't rely on any functionality from the ManifestService. ImageDigestCache should become ManifestServiceCache (or potentially RegistryServiceClientCache, since that is the only class making the single network call). We should experiment with combining ManifestService and RegistryServiceClient.

Also, ManifestServiceExtensions can be combined into the ManifestService class since they are in the same namespace.

I have put together a branch where I experimented with removing the DockerService dependency on the ManifestService, but it is currently incomplete and not fully functional - https://github.com/lbussell/docker-tools/commits/dev/loganbussell/1262playground/

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[lbussell]

[Triage] We may want to consider keeping the ManifestServiceExtensions class, since its existence allows us to keep the IManifestService interface as small as possible which reduces the complexity of testing the ManifestService. The methods in the ManifestServiceExtensions don't rely on any of the ManifestService's internal state.