Skip to content

Commit

Permalink
refactor for simplicity
Browse files Browse the repository at this point in the history
  • Loading branch information
asilverman committed Oct 12, 2023
1 parent fea7164 commit 4480e4e
Show file tree
Hide file tree
Showing 14 changed files with 36 additions and 27 deletions.
3 changes: 1 addition & 2 deletions src/Bicep.Cli/Commands/PublishCommand.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
using Bicep.Core.Features;
using Bicep.Core.FileSystem;
using Bicep.Core.Registry;
using Bicep.Core.Registry.Oci;
using Bicep.Core.SourceCode;

namespace Bicep.Cli.Commands
Expand Down Expand Up @@ -99,7 +98,7 @@ private async Task PublishModuleAsync(ArtifactReference target, Stream compiledA

private ArtifactReference ValidateReference(string targetModuleReference, Uri targetModuleUri)
{
if (!this.moduleDispatcher.TryGetArtifactReference(targetModuleReference, "module", targetModuleUri).IsSuccess(out var moduleReference, out var failureBuilder))
if (!this.moduleDispatcher.TryGetModuleReference(targetModuleReference, targetModuleUri).IsSuccess(out var moduleReference, out var failureBuilder))
{
// TODO: We should probably clean up the dispatcher contract so this sort of thing isn't necessary (unless we change how target module is set in this command)
var message = failureBuilder(DiagnosticBuilder.ForDocumentStart()).Message;
Expand Down
8 changes: 4 additions & 4 deletions src/Bicep.Core.IntegrationTests/RegistryTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ public async Task ModuleRestoreContentionShouldProduceConsistentState(bool publi

var moduleReferences = dataSet.RegistryModules.Values
.OrderBy(m => m.Metadata.Target)
.Select(m => dispatcher.TryGetArtifactReference(m.Metadata.Target, "module", RandomFileUri()).IsSuccess(out var @ref) ? @ref : throw new AssertFailedException($"Invalid module target '{m.Metadata.Target}'."))
.Select(m => dispatcher.TryGetModuleReference(m.Metadata.Target, RandomFileUri()).IsSuccess(out var @ref) ? @ref : throw new AssertFailedException($"Invalid module target '{m.Metadata.Target}'."))
.ToImmutableList();

moduleReferences.Should().HaveCount(7);
Expand Down Expand Up @@ -238,7 +238,7 @@ public async Task ModuleRestoreWithStuckFileLockShouldFailAfterTimeout(IEnumerab
var configuration = BicepTestConstants.BuiltInConfigurationWithAllAnalyzersDisabled;
var moduleReferences = moduleInfos
.OrderBy(m => m.Metadata.Target)
.Select(m => dispatcher.TryGetArtifactReference(m.Metadata.Target, "module", RandomFileUri()).IsSuccess(out var @ref) ? @ref : throw new AssertFailedException($"Invalid module target '{m.Metadata.Target}'."))
.Select(m => dispatcher.TryGetModuleReference(m.Metadata.Target, RandomFileUri()).IsSuccess(out var @ref) ? @ref : throw new AssertFailedException($"Invalid module target '{m.Metadata.Target}'."))
.ToImmutableList();

moduleReferences.Should().HaveCount(moduleCount);
Expand Down Expand Up @@ -307,7 +307,7 @@ public async Task ForceModuleRestoreWithStuckFileLockShouldFailAfterTimeout(IEnu

var moduleReferences = moduleInfos
.OrderBy(m => m.Metadata.Target)
.Select(m => dispatcher.TryGetArtifactReference(m.Metadata.Target, "module", RandomFileUri()).IsSuccess(out var @ref) ? @ref : throw new AssertFailedException($"Invalid module target '{m.Metadata.Target}'."))
.Select(m => dispatcher.TryGetModuleReference(m.Metadata.Target, RandomFileUri()).IsSuccess(out var @ref) ? @ref : throw new AssertFailedException($"Invalid module target '{m.Metadata.Target}'."))
.ToImmutableList();

moduleReferences.Should().HaveCount(moduleCount);
Expand Down Expand Up @@ -384,7 +384,7 @@ public async Task ForceModuleRestoreShouldRestoreAllModules(IEnumerable<External
var configuration = BicepTestConstants.BuiltInConfigurationWithAllAnalyzersDisabled;
var moduleReferences = moduleInfos
.OrderBy(m => m.Metadata.Target)
.Select(m => dispatcher.TryGetArtifactReference(m.Metadata.Target, "module", RandomFileUri()).IsSuccess(out var @ref) ? @ref : throw new AssertFailedException($"Invalid module target '{m.Metadata.Target}'."))
.Select(m => dispatcher.TryGetModuleReference(m.Metadata.Target, RandomFileUri()).IsSuccess(out var @ref) ? @ref : throw new AssertFailedException($"Invalid module target '{m.Metadata.Target}'."))
.ToImmutableList();

moduleReferences.Should().HaveCount(moduleCount);
Expand Down
6 changes: 3 additions & 3 deletions src/Bicep.Core.Samples/DataSetsExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ public static Mock<IContainerRegistryClientFactory> CreateMockRegistryClients(Im
{
var target = publishInfo.Metadata.Target;

if (!dispatcher.TryGetArtifactReference(target, "module", RandomFileUri()).IsSuccess(out var @ref) || @ref is not OciArtifactReference targetReference)
if (!dispatcher.TryGetModuleReference(target, RandomFileUri()).IsSuccess(out var @ref) || @ref is not OciArtifactReference targetReference)
{
throw new InvalidOperationException($"Module '{moduleName}' has an invalid target reference '{target}'. Specify a reference to an OCI artifact.");
}
Expand Down Expand Up @@ -149,7 +149,7 @@ public static ITemplateSpecRepositoryFactory CreateMockTemplateSpecRepositoryFac

foreach (var (moduleName, templateSpecInfo) in templateSpecs)
{
if (!dispatcher.TryGetArtifactReference(templateSpecInfo.Metadata.Target, "module", RandomFileUri()).IsSuccess(out var @ref) || @ref is not TemplateSpecModuleReference reference)
if (!dispatcher.TryGetModuleReference(templateSpecInfo.Metadata.Target, RandomFileUri()).IsSuccess(out var @ref) || @ref is not TemplateSpecModuleReference reference)
{
throw new InvalidOperationException($"Module '{moduleName}' has an invalid target reference '{templateSpecInfo.Metadata.Target}'. Specify a reference to a template spec.");
}
Expand Down Expand Up @@ -191,7 +191,7 @@ public static async Task PublishModuleToRegistryAsync(IContainerRegistryClientFa
.AddSingleton(featureProviderFactory)
).Construct<IModuleDispatcher>();

var targetReference = dispatcher.TryGetArtifactReference(target, "TODO", RandomFileUri()).IsSuccess(out var @ref) ? @ref
var targetReference = dispatcher.TryGetModuleReference(target, RandomFileUri()).IsSuccess(out var @ref) ? @ref
: throw new InvalidOperationException($"Module '{moduleName}' has an invalid target reference '{target}'. Specify a reference to an OCI artifact.");

var result = CompilationHelper.Compile(moduleSource);
Expand Down
8 changes: 4 additions & 4 deletions src/Bicep.Core.UnitTests/Registry/ArtifactDispatcherTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -89,20 +89,20 @@ public async Task MockRegistries_ModuleLifecycle()

var validRefUri = RandomFileUri();
ArtifactReference? validRef = new MockModuleReference("validRef", validRefUri);
mock.Setup(m => m.TryParseArtifactReference(null, "module", "validRef", validRefUri)).Returns(ResultHelper.Create(validRef, @null));
mock.Setup(m => m.TryParseArtifactReference(null, ArtifactType.Module, "validRef")).Returns(ResultHelper.Create(validRef, @null));

var validRefUri2 = RandomFileUri();
ArtifactReference? validRef2 = new MockModuleReference("validRef2", validRefUri2);
mock.Setup(m => m.TryParseArtifactReference(null, "module", "validRef2", validRefUri2)).Returns(ResultHelper.Create(validRef2, @null));
mock.Setup(m => m.TryParseArtifactReference(null, ArtifactType.Module, "validRef2")).Returns(ResultHelper.Create(validRef2, @null));

var validRefUri3 = RandomFileUri();
ArtifactReference? validRef3 = new MockModuleReference("validRef3", validRefUri3);
mock.Setup(m => m.TryParseArtifactReference(null, "module", "validRef3", validRefUri3)).Returns(ResultHelper.Create(validRef3, @null));
mock.Setup(m => m.TryParseArtifactReference(null, ArtifactType.Module, "validRef3")).Returns(ResultHelper.Create(validRef3, @null));

var badRefUri = RandomFileUri();
ArtifactReference? nullRef = null;
ErrorBuilderDelegate? badRefError = x => new ErrorDiagnostic(x.TextSpan, "BCPMock", "Bad ref error");
mock.Setup(m => m.TryParseArtifactReference(null, "module", "badRef", badRefUri)).Returns(ResultHelper.Create(nullRef, badRefError));
mock.Setup(m => m.TryParseArtifactReference(null, ArtifactType.Module, "badRef")).Returns(ResultHelper.Create(nullRef, badRefError));

mock.Setup(m => m.IsArtifactRestoreRequired(validRef)).Returns(true);
mock.Setup(m => m.IsArtifactRestoreRequired(validRef2)).Returns(false);
Expand Down
4 changes: 2 additions & 2 deletions src/Bicep.Core/Diagnostics/DiagnosticBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ public ErrorDiagnostic DirectAccessToCollectionNotSupported(IEnumerable<string>?
"BCP162",
"Expected a loop item variable identifier or \"(\" at this location.");

public ErrorDiagnostic ScopeUnsupportedOnChildResource(string parentIdentifier) => new(
public ErrorDiagnostic ScopeUnsupportedOnChildResource() => new(
TextSpan,
"BCP164",
$"A child resource's scope is computed based on the scope of its ancestor resource. This means that using the \"{LanguageConstants.ResourceScopePropertyName}\" property on a child resource is unsupported.");
Expand Down Expand Up @@ -1761,7 +1761,7 @@ public ErrorDiagnostic IndexOutOfBounds(string typeName, long tupleLength, long
var message = new StringBuilder("The provided index value of \"").Append(indexSought).Append("\" is not valid for type \"").Append(typeName).Append("\".");
if (tupleLength > 0)
{
message.Append(" Indexes for this type must be between 0 and ").Append(tupleLength - 1).Append(".");
message.Append(" Indexes for this type must be between 0 and ").Append(tupleLength - 1).Append('.');
}

return new(TextSpan, "BCP311", message.ToString());
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Emit/ScopeHelper.cs
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ void logInvalidScopeDiagnostic(IPositionable positionable, ResourceScope supplie
if (resource.TryGetScopeSyntax() is { } scopeSyntax)
{
// it doesn't make sense to have scope on a descendent resource; it should be inherited from the oldest ancestor.
diagnosticWriter.Write(scopeSyntax, x => x.ScopeUnsupportedOnChildResource(ancestors.Last().Resource.Symbol.Name));
diagnosticWriter.Write(scopeSyntax, x => x.ScopeUnsupportedOnChildResource());
// TODO: format the ancestor name using the resource accessor (::) for nested resources
scopeInfo[resource] = defaultScopeData;
continue;
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Registry/ArtifactRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public abstract class ArtifactRegistry<T> : IArtifactRegistry where T : Artifact

public abstract ResultWithDiagnostic<Uri> TryGetLocalArtifactEntryPointUri(T reference);

public abstract ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? aliasName, string artifactType, string reference, Uri parentModuleUri);
public abstract ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? aliasName, ArtifactType artifactType, string reference);

public abstract string? TryGetDocumentationUri(T reference);

Expand Down
2 changes: 2 additions & 0 deletions src/Bicep.Core/Registry/IArtifactReferenceFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,5 +15,7 @@ public interface IArtifactReferenceFactory

ResultWithDiagnostic<ArtifactReference> TryGetArtifactReference(string reference, string artifactType, Uri parentModuleUri);

ResultWithDiagnostic<ArtifactReference> TryGetModuleReference(string reference, Uri parentModuleUri);

ResultWithDiagnostic<ArtifactReference> TryGetArtifactReference(IArtifactReferenceSyntax artifactDeclaration, Uri parentModuleUri);
}
8 changes: 6 additions & 2 deletions src/Bicep.Core/Registry/IArtifactRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,11 @@

namespace Bicep.Core.Registry
{
public enum ArtifactType
{
Module,
Provider,
}
/// <summary>
/// An implementation of a Bicep artifact registry.
/// </summary>
Expand All @@ -34,8 +39,7 @@ public interface IArtifactRegistry
/// <param name="aliasName">The alias name</param>
/// <param name="reference">The unqualified artifact reference</param>
/// <param name="artifactType">The artifact type. Either "module" or "provider"</param>
/// <param name="parentModuleUri">The URI of the parent module</param>
ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? aliasName, string artifactType, string reference, Uri parentModuleUri);
ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? aliasName, ArtifactType artifactType, string reference);

/// <summary>
/// Returns true if the specified artifact is already cached in the local cache.
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Registry/LocalModuleRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ public LocalModuleRegistry(IFileResolver fileResolver, Uri parentModuleUri, Bice

public override RegistryCapabilities GetCapabilities(LocalModuleReference reference) => RegistryCapabilities.Default;

public override ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? alias, string artifactType, string reference, Uri parentModuleUri)
public override ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? alias, ArtifactType artifactType, string reference)
{
if (!LocalModuleReference.TryParse(reference, parentModuleUri).IsSuccess(out var @ref, out var failureBuilder))
{
Expand Down
8 changes: 6 additions & 2 deletions src/Bicep.Core/Registry/ModuleDispatcher.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ private ImmutableDictionary<string, IArtifactRegistry> Registries(Uri parentModu
public ImmutableArray<string> AvailableSchemes(Uri parentModuleUri)
=> Registries(parentModuleUri).Keys.OrderBy(s => s).ToImmutableArray();

public ResultWithDiagnostic<ArtifactReference> TryGetModuleReference(string reference, Uri parentModuleUri)
=> TryGetArtifactReference(reference, "module", parentModuleUri);


public ResultWithDiagnostic<ArtifactReference> TryGetArtifactReference(string reference, string artifactType, Uri parentModuleUri)
{
var registries = Registries(parentModuleUri);
Expand All @@ -51,7 +55,7 @@ public ResultWithDiagnostic<ArtifactReference> TryGetArtifactReference(string re
// local path reference
if (registries.TryGetValue(ModuleReferenceSchemes.Local, out var localRegistry))
{
return localRegistry.TryParseArtifactReference(null, "module", parts[0], parentModuleUri);
return localRegistry.TryParseArtifactReference(null, ArtifactType.Module, parts[0]);
}

return new(x => x.UnknownModuleReferenceScheme(ModuleReferenceSchemes.Local, this.AvailableSchemes(parentModuleUri)));
Expand All @@ -73,7 +77,7 @@ public ResultWithDiagnostic<ArtifactReference> TryGetArtifactReference(string re
// the scheme is recognized
var rawValue = parts[1];

return registry.TryParseArtifactReference(aliasName, artifactType, rawValue, parentModuleUri);
return registry.TryParseArtifactReference(aliasName, ArtifactType.Module, rawValue);
}

// unknown scheme
Expand Down
6 changes: 3 additions & 3 deletions src/Bicep.Core/Registry/OciArtifactRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,12 @@ public override RegistryCapabilities GetCapabilities(OciArtifactReference refere
return reference.Tag is null ? RegistryCapabilities.Default : RegistryCapabilities.Publish;
}

public override ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? aliasName, string artifactType, string reference, Uri parentModuleUri)
public override ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? aliasName, ArtifactType artifactType, string reference)
{
var type = artifactType switch
{
"module" => OciArtifactReferenceType.Module,
"provider" => OciArtifactReferenceType.Provider,
ArtifactType.Module => OciArtifactReferenceType.Module,
ArtifactType.Provider => OciArtifactReferenceType.Provider,
_ => throw new ArgumentException($"Unexpected artifact type \"{artifactType}\"."),
};
if (!OciArtifactReference.TryParse(type, aliasName, reference, configuration, parentModuleUri).IsSuccess(out var @ref, out var failureBuilder))
Expand Down
2 changes: 1 addition & 1 deletion src/Bicep.Core/Registry/TemplateSpecModuleRegistry.cs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public TemplateSpecModuleRegistry(IFileResolver fileResolver, ITemplateSpecRepos

public override RegistryCapabilities GetCapabilities(TemplateSpecModuleReference reference) => RegistryCapabilities.Default;

public override ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? aliasName, string artifactType, string reference, Uri parentModuleUri)
public override ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? aliasName, ArtifactType artifactType, string reference)
{
if (!TemplateSpecModuleReference.TryParse(aliasName, reference, configuration, parentModuleUri).IsSuccess(out var @ref, out var failureBuilder))
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,7 @@ public ResultWithDiagnostic<Uri> TryGetLocalArtifactEntryPointUri(ArtifactRefere

public Task<string?> TryGetDescription(ArtifactReference reference) => Task.FromResult<string?>(null);

public ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? aliasName,string artifactType, string reference, Uri parentModuleUri)
public ResultWithDiagnostic<ArtifactReference> TryParseArtifactReference(string? _, ArtifactType artifactType, string reference)
{
return new(new MockModuleRef(reference, PathHelper.FilePathToFileUrl(Path.GetTempFileName())));
}
Expand Down

0 comments on commit 4480e4e

Please sign in to comment.