Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 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
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,16 @@

var db2 = sql2.AddDatabase("db2");

var dbsetup = builder.AddProject<Projects.SqlServerEndToEnd_DbSetup>("dbsetup")
.WithReference(db1).WaitFor(sql1)
.WithReference(db2).WaitFor(sql2);
//var dbsetup = builder.AddProject<Projects.SqlServerEndToEnd_DbSetup>("dbsetup")
// .WithReference(db1).WaitFor(sql1)
// .WithReference(db2).WaitFor(sql2);

builder.AddProject<Projects.SqlServerEndToEnd_ApiService>("api")
.WithExternalHttpEndpoints()
.WithReference(db1).WaitFor(db1)
.WithReference(db2).WaitFor(db2)
.WaitForCompletion(dbsetup);
//.WaitForCompletion(dbsetup)
;

#if !SKIP_DASHBOARD_REFERENCE
// This project is only added in playground projects to support development/debugging
Expand Down
82 changes: 80 additions & 2 deletions src/Aspire.Hosting.SqlServer/SqlServerBuilderExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@

using Aspire.Hosting;
using Aspire.Hosting.ApplicationModel;
using Microsoft.Data.SqlClient;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;

namespace Aspire.Hosting;

Expand Down Expand Up @@ -45,6 +47,42 @@ public static IResourceBuilder<SqlServerServerResource> AddSqlServer(this IDistr
}
});

builder.Eventing.Subscribe<ResourceReadyEvent>(sqlServer, async (@event, ct) =>
{
if (connectionString is null)
{
throw new DistributedApplicationException($"ResourceReadyEvent was published for the '{sqlServer.Name}' resource but the connection string was null.");
}

using var sqlConnection = new SqlConnection(connectionString);
await sqlConnection.OpenAsync(ct).ConfigureAwait(false);

foreach (var sqlDatabase in sqlServer.DatabaseResources)
{
var quotedDatabaseIdentifier = new SqlCommandBuilder().QuoteIdentifier(sqlDatabase.DatabaseName);

try
{
if (sqlConnection.State != System.Data.ConnectionState.Open)
{
throw new InvalidOperationException($"Could not open connection to '{sqlServer.Name}'");
}

var scriptAnnotation = sqlDatabase.Annotations.OfType<ScriptAnnotation>().LastOrDefault();

using var command = sqlConnection.CreateCommand();
command.CommandText = scriptAnnotation?.Script ??
$"IF ( NOT EXISTS ( SELECT 1 FROM sys.databases WHERE name = @DatabaseName ) ) CREATE DATABASE {quotedDatabaseIdentifier};";
Copy link
Member

Choose a reason for hiding this comment

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

Why the difference between the name in the WHERE and the name used on the Create Database? Shouldn't they both use the same SqlParameter?

Copy link
Member Author

Choose a reason for hiding this comment

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

Parameters can't be used for object names (such as database names, table names, or column names).
It's used in the query part because in that case it's a string value.

command.Parameters.Add(new SqlParameter("@DatabaseName", sqlDatabase.DatabaseName));
await command.ExecuteNonQueryAsync(ct).ConfigureAwait(false);
}
catch (Exception e)
{
@event.Services.GetRequiredService<ILogger<SqlServerServerResource>>().LogError(e, "Failed to create database '{DatabaseName}'", sqlDatabase.DatabaseName);
Copy link
Member

Choose a reason for hiding this comment

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

Can we log this to resource's logger? Where does this currently log? To the console of the AppHost?

Copy link
Member Author

Choose a reason for hiding this comment

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

It currently shows up in the console. Would it be nice if it were in the database resource log in the dashboard? If so what is the way to do it?

}
}
});

var healthCheckKey = $"{name}_check";
builder.Services.AddHealthChecks().AddSqlServer(sp => connectionString ?? throw new InvalidOperationException("Connection string is unavailable"), name: healthCheckKey);

Expand Down Expand Up @@ -72,12 +110,33 @@ public static IResourceBuilder<SqlServerDatabaseResource> AddDatabase(this IReso
ArgumentNullException.ThrowIfNull(builder);
ArgumentException.ThrowIfNullOrEmpty(name);

Dictionary<string, string> databaseConnectionStrings = [];

// Use the resource name as the database name if it's not provided
databaseName ??= name;

builder.Resource.AddDatabase(name, databaseName);
var sqlServerDatabase = new SqlServerDatabaseResource(name, databaseName, builder.Resource);
return builder.ApplicationBuilder.AddResource(sqlServerDatabase);

builder.Resource.AddDatabase(sqlServerDatabase);

builder.ApplicationBuilder.Eventing.Subscribe<ConnectionStringAvailableEvent>(sqlServerDatabase, async (@event, ct) =>
{
var databaseConnectionString = await sqlServerDatabase.ConnectionStringExpression.GetValueAsync(ct).ConfigureAwait(false);

if (databaseConnectionString == null)
{
throw new DistributedApplicationException($"ConnectionStringAvailableEvent was published for the '{name}' resource but the connection string was null.");
}

databaseConnectionStrings[name] = databaseConnectionString;
});

var healthCheckKey = $"{name}_check";
builder.ApplicationBuilder.Services.AddHealthChecks().AddSqlServer(sp => databaseConnectionStrings[name] ?? throw new InvalidOperationException("Connection string is unavailable"), name: healthCheckKey);

return builder.ApplicationBuilder
.AddResource(sqlServerDatabase)
.WithHealthCheck(healthCheckKey);
}

/// <summary>
Expand Down Expand Up @@ -112,4 +171,23 @@ public static IResourceBuilder<SqlServerServerResource> WithDataBindMount(this I

return builder.WithBindMount(source, "/var/opt/mssql", isReadOnly);
}

/// <summary>
/// Alters the JSON configuration document used by the emulator.
/// </summary>
/// <param name="builder">The builder for the <see cref="SqlServerDatabaseResource"/>.</param>
/// <param name="script">The SQL script used to create the database.</param>
/// <returns>A reference to the <see cref="IResourceBuilder{T}"/>.</returns>
/// <remarks>
/// <value>Default script is <code>IF ( NOT EXISTS ( SELECT 1 FROM sys.databases WHERE name = @DatabaseName ) ) CREATE DATABASE [&lt;QUOTED_DATABASE_NAME%gt;];</code></value>
/// </remarks>
public static IResourceBuilder<SqlServerDatabaseResource> WithCreationScript(this IResourceBuilder<SqlServerDatabaseResource> builder, string script)
{
ArgumentNullException.ThrowIfNull(builder);
ArgumentNullException.ThrowIfNull(script);

builder.WithAnnotation(new ScriptAnnotation(script));

return builder;
}
}
8 changes: 6 additions & 2 deletions src/Aspire.Hosting.SqlServer/SqlServerServerResource.cs
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,18 @@ public ReferenceExpression ConnectionStringExpression
}

private readonly Dictionary<string, string> _databases = new(StringComparers.ResourceName);
private readonly List<SqlServerDatabaseResource> _databaseResources = [];

/// <summary>
/// A dictionary where the key is the resource name and the value is the database name.
/// </summary>
public IReadOnlyDictionary<string, string> Databases => _databases;

internal void AddDatabase(string name, string databaseName)
internal void AddDatabase(SqlServerDatabaseResource database)
{
_databases.TryAdd(name, databaseName);
_databases.TryAdd(database.Name, database.DatabaseName);
_databaseResources.Add(database);
}

internal IReadOnlyList<SqlServerDatabaseResource> DatabaseResources => _databaseResources;
}
25 changes: 25 additions & 0 deletions src/Aspire.Hosting/ApplicationModel/ScriptAnnotation.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace Aspire.Hosting.ApplicationModel;

/// <summary>
/// Represents an annotation for defining a script to create a resource.
/// </summary>
public sealed class ScriptAnnotation : IResourceAnnotation
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should give this a more specific name. ScriptAnnotation sounds really general.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was torn between a database specific name or not. But in the end it's only a string. Could have been all the way to StringAnnotation, so I understand why a more specific one would make sense. If we tell users they can add one or change it. Another advantage of a specific name is that it won't conflict with another annotation that would use the same type for something else, and we would be screwed. So I agree with that: DatabaseCreationScriptAnnotation for instance.

NB: To go all the way meta we could have general annotation like KeyValuePairAnnotation which would also have a key/value ("databasecreationscript"/"create database [foo]"). But that's why we have custom type for ;)

Copy link
Member

Choose a reason for hiding this comment

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

Given the summary of the class "for defining a script to create a resource." I thought the class would be named more specific.

Copy link
Member Author

Choose a reason for hiding this comment

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

Changed to CreationScriptAnnotation since there should be only on in a resource, whatever resource is using it, so there can't be a conflict.

Copy link
Member

Choose a reason for hiding this comment

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

Thinking more about this, we also have a sample for how to customize the initial database:

https://github.com/dotnet/aspire-samples/blob/fc7aa13409821eb1d7b7ccd374b465c97dca5591/samples/DatabaseContainers/DatabaseContainers.AppHost/Program.cs#L38-L45

  1. Does this feature work well with this example?
  2. I wonder if we really need to support a custom create database script, and instead point people to this route. (possibly with an option for turning this feature off).

cc @DamianEdwards

Copy link
Member Author

Choose a reason for hiding this comment

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

As a user I would definitely prefer this extension method (and annotation) to customize the create database script than having to deal with bind mounts and understand how the container is configured. The same way we have WithDataVolume extensions. Or should we provide (in addition maybe) a method to configure this "/usr/config/entrypoint.sh" script in sql server container to make it easier?

Though I would understand if we say we skip this feature (customizing CREATE DATABASE) since there is already a way to do it in the container itself.

{
/// <summary>
/// Initializes a new instance of the <see cref="ScriptAnnotation"/> class.
/// </summary>
/// <param name="script">The script used to create the resource.</param>
public ScriptAnnotation(string script)
{
ArgumentNullException.ThrowIfNull(script);
Script = script;
}

/// <summary>
/// Gets the script used to create the resource.
/// </summary>
public string Script { get; }
}
89 changes: 75 additions & 14 deletions tests/Aspire.Hosting.SqlServer.Tests/SqlServerFunctionalTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ public async Task VerifyWaitForOnSqlServerBlocksDependentResources()
[RequiresDocker]
public async Task VerifySqlServerResource()
{
const string databaseName = "newdb";

var cts = new CancellationTokenSource(TimeSpan.FromMinutes(5));
var pipeline = new ResiliencePipelineBuilder()
.AddRetry(new() { MaxRetryAttempts = int.MaxValue, BackoffType = DelayBackoffType.Linear, Delay = TimeSpan.FromSeconds(2) })
Expand All @@ -68,18 +70,18 @@ public async Task VerifySqlServerResource()
using var builder = TestDistributedApplicationBuilder.Create(o => { }, testOutputHelper);

var sqlserver = builder.AddSqlServer("sqlserver");
var tempDb = sqlserver.AddDatabase("tempdb");
var newDb = sqlserver.AddDatabase(databaseName);

using var app = builder.Build();

await app.StartAsync(cts.Token);

var hb = Host.CreateApplicationBuilder();

hb.Configuration[$"ConnectionStrings:{tempDb.Resource.Name}"] = await tempDb.Resource.ConnectionStringExpression.GetValueAsync(default);
hb.Configuration[$"ConnectionStrings:{newDb.Resource.Name}"] = await newDb.Resource.ConnectionStringExpression.GetValueAsync(default);

hb.AddSqlServerDbContext<TestDbContext>(tempDb.Resource.Name);
hb.AddSqlServerClient(tempDb.Resource.Name);
hb.AddSqlServerDbContext<TestDbContext>(newDb.Resource.Name);
hb.AddSqlServerClient(newDb.Resource.Name);

using var host = hb.Build();

Expand Down Expand Up @@ -119,9 +121,11 @@ await pipeline.ExecuteAsync(async token =>
[RequiresDocker]
public async Task WithDataShouldPersistStateBetweenUsages(bool useVolume)
{
const string databaseName = "db";

string? volumeName = null;
string? bindMountPath = null;

var cts = new CancellationTokenSource(TimeSpan.FromMinutes(10));
var pipeline = new ResiliencePipelineBuilder()
.AddRetry(new() { MaxRetryAttempts = int.MaxValue, BackoffType = DelayBackoffType.Linear, Delay = TimeSpan.FromSeconds(2) })
Expand All @@ -132,7 +136,7 @@ public async Task WithDataShouldPersistStateBetweenUsages(bool useVolume)
using var builder1 = TestDistributedApplicationBuilder.Create(o => { }, testOutputHelper);

var sqlserver1 = builder1.AddSqlServer("sqlserver");
var masterdb1 = sqlserver1.AddDatabase("master");
var db1 = sqlserver1.AddDatabase(databaseName);
Copy link
Member Author

Choose a reason for hiding this comment

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

Not using master updates this test to ensure the database is created. master was used since it's a default one in sql server.


var password = sqlserver1.Resource.PasswordParameter.Value;

Expand All @@ -149,7 +153,7 @@ public async Task WithDataShouldPersistStateBetweenUsages(bool useVolume)
{
bindMountPath = Path.Combine(Path.GetTempPath(), Path.GetRandomFileName());

Directory.CreateDirectory(bindMountPath);
var directoryInfo = Directory.CreateDirectory(bindMountPath);

if (!OperatingSystem.IsWindows())
{
Expand All @@ -171,18 +175,18 @@ public async Task WithDataShouldPersistStateBetweenUsages(bool useVolume)

await app1.StartAsync();

await app1.ResourceNotifications.WaitForResourceHealthyAsync(masterdb1.Resource.Name, cts.Token);
await app1.ResourceNotifications.WaitForResourceHealthyAsync(db1.Resource.Name, cts.Token);

try
{
var hb1 = Host.CreateApplicationBuilder();

hb1.Configuration.AddInMemoryCollection(new Dictionary<string, string?>
{
[$"ConnectionStrings:{masterdb1.Resource.Name}"] = await masterdb1.Resource.ConnectionStringExpression.GetValueAsync(default),
[$"ConnectionStrings:{db1.Resource.Name}"] = await db1.Resource.ConnectionStringExpression.GetValueAsync(default),
});

hb1.AddSqlServerClient(masterdb1.Resource.Name);
hb1.AddSqlServerClient(db1.Resource.Name);

using var host1 = hb1.Build();

Expand Down Expand Up @@ -239,7 +243,7 @@ await pipeline.ExecuteAsync(async token =>
var passwordParameter2 = builder2.AddParameter("pwd", password);

var sqlserver2 = builder2.AddSqlServer("sqlserver2", passwordParameter2);
var masterdb2 = sqlserver2.AddDatabase("master");
var db2 = sqlserver2.AddDatabase(databaseName);

if (useVolume)
{
Expand All @@ -254,18 +258,18 @@ await pipeline.ExecuteAsync(async token =>
{
await app2.StartAsync();

await app2.ResourceNotifications.WaitForResourceHealthyAsync(masterdb2.Resource.Name, cts.Token);
await app2.ResourceNotifications.WaitForResourceHealthyAsync(db2.Resource.Name, cts.Token);

try
{
var hb2 = Host.CreateApplicationBuilder();

hb2.Configuration.AddInMemoryCollection(new Dictionary<string, string?>
{
[$"ConnectionStrings:{masterdb2.Resource.Name}"] = await masterdb2.Resource.ConnectionStringExpression.GetValueAsync(default),
[$"ConnectionStrings:{db2.Resource.Name}"] = await db2.Resource.ConnectionStringExpression.GetValueAsync(default),
});

hb2.AddSqlServerClient(masterdb2.Resource.Name);
hb2.AddSqlServerClient(db2.Resource.Name);

using (var host2 = hb2.Build())
{
Expand Down Expand Up @@ -319,4 +323,61 @@ await pipeline.ExecuteAsync(async token =>
}
}
}

[Fact]
[RequiresDocker]
public async Task AddDatabaseCreatesDatabaseWithCustomScript()
{
const string databaseName = "newdb";

var cts = new CancellationTokenSource(TimeSpan.FromMinutes(5));
var pipeline = new ResiliencePipelineBuilder()
.AddRetry(new() { MaxRetryAttempts = 10, BackoffType = DelayBackoffType.Linear, Delay = TimeSpan.FromSeconds(2) })
.Build();

using var builder = TestDistributedApplicationBuilder.Create(o => { }, testOutputHelper);

var sqlserver = builder.AddSqlServer("sqlserver");

// Create a datbase with Accent Insensitive collation
var newDb = sqlserver.AddDatabase(databaseName)
.WithCreationScript($"CREATE DATABASE [{databaseName}] COLLATE French_CI_AI;");

using var app = builder.Build();

await app.StartAsync(cts.Token);

var hb = Host.CreateApplicationBuilder();

hb.Configuration[$"ConnectionStrings:{newDb.Resource.Name}"] = await newDb.Resource.ConnectionStringExpression.GetValueAsync(default);

hb.AddSqlServerClient(newDb.Resource.Name);

using var host = hb.Build();

await host.StartAsync();

await app.ResourceNotifications.WaitForResourceHealthyAsync(newDb.Resource.Name, cts.Token);

// Test SqlConnection
await pipeline.ExecuteAsync(async token =>
{
var conn = host.Services.GetRequiredService<SqlConnection>();

if (conn.State != System.Data.ConnectionState.Open)
{
await conn.OpenAsync(token);
}

var selectCommand = conn.CreateCommand();
selectCommand.CommandText = """
CREATE TABLE [Modèles] ([Name] nvarchar(max) NOT NULL);
INSERT INTO [Modèles] ([Name]) VALUES ('BatMobile');
SELECT * FROM [Modeles]; -- Incorrect accent to verify the database collation
""";

var results = await selectCommand.ExecuteReaderAsync(token);
Assert.True(results.HasRows);
}, cts.Token);
}
}
Loading