Add SurrealDB hosting/client integration

[Odonno]

Closes #<ISSUE_NUMBER>

PR Checklist

• Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
• Based off latest main branch of toolkit
• PR doesn't include merge commits (always rebase on top of our main, if needed)
• New integration
  • Docs are written
  • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
• Tests for the changes have been added (for bug fixes / features) (if applicable)
• Contains NO breaking changes
• Every new API (including internal ones) has full XML docs
• Code follows all style conventions

Other information

[Copilot]

Copilot reviewed 30 out of 45 changed files in this pull request and generated 1 comment.

Files not reviewed (15)

• CODEOWNERS: Language not supported
• CommunityToolkit.Aspire.sln: Language not supported
• Directory.Packages.props: Language not supported
• examples/surrealdb/CommunityToolkit.Aspire.Hosting.SurrealDb.ApiService/CommunityToolkit.Aspire.Hosting.SurrealDb.ApiService.csproj: Language not supported
• examples/surrealdb/CommunityToolkit.Aspire.Hosting.SurrealDb.ApiService/Properties/launchSettings.json: Language not supported
• examples/surrealdb/CommunityToolkit.Aspire.Hosting.SurrealDb.ApiService/appsettings.json: Language not supported
• examples/surrealdb/CommunityToolkit.Aspire.Hosting.SurrealDb.AppHost/CommunityToolkit.Aspire.Hosting.SurrealDb.AppHost.csproj: Language not supported
• examples/surrealdb/CommunityToolkit.Aspire.Hosting.SurrealDb.AppHost/Properties/launchSettings.json: Language not supported
• examples/surrealdb/CommunityToolkit.Aspire.Hosting.SurrealDb.AppHost/appsettings.json: Language not supported
• examples/surrealdb/CommunityToolkit.Aspire.Hosting.SurrealDb.ServiceDefaults/CommunityToolkit.Aspire.Hosting.SurrealDb.ServiceDefaults.csproj: Language not supported
• src/CommunityToolkit.Aspire.Hosting.SurrealDb/CommunityToolkit.Aspire.Hosting.SurrealDb.csproj: Language not supported
• src/CommunityToolkit.Aspire.Hosting.SurrealDb/PublicAPI.Shipped.txt: Language not supported
• src/CommunityToolkit.Aspire.Hosting.SurrealDb/PublicAPI.Unshipped.txt: Language not supported
• examples/surrealdb/CommunityToolkit.Aspire.Hosting.SurrealDb.AppHost/Program.cs: Evaluated as low risk
• README.md: Evaluated as low risk

Comments suppressed due to low confidence (2)

examples/surrealdb/CommunityToolkit.Aspire.Hosting.SurrealDb.ApiService/Program.cs:10

• [nitpick] The value 'CamelCase' for NamingPolicy might be ambiguous. Consider using a predefined constant or enum if available.

settings.Options!.NamingPolicy = "CamelCase";

examples/surrealdb/CommunityToolkit.Aspire.Hosting.SurrealDb.ApiService/Models/WeatherForecast.cs:10

• [nitpick] The constant name 'Table' should follow a consistent naming convention. Consider renaming it to 'TableName' or 'TableIdentifier'.

internal const string Table = "weatherForecast";

[aaronpowell]

Having a look into the test failure, and it's because they are timing out, which made me think that the problem is to do with the container not starting. This is... sort of true, the container used for the tests does start, but it immediately exists because it's missing the args to tell Surreal to actually start.

In this block https://github.com/CommunityToolkit/Aspire/pull/365/files#diff-bae88f8ce8a5624289417d505a6ba0a190e20d70c801f6da997ae6121820b481R50-R56

You'll need to add WithCommand("start", "memory") (I don't think you need the entrypoint too) and then it gets onto the next place where the tests are failing.

The next place it seems to fail is that it can't resolve an IHttpClientFactory, which looking at the SurrealDbClient type is a required constructor argument.

My guess is that for the ConfromanceTests we will need to manually register that type (it'd be there in the API project, which is why it's not surfaced as an issue), and probably for the others in the SurrealDB client integration.

Looking at how you can register SurrealDB via their native SDK, I see you can pass a type argument that implements ISurrealDbClient, would it make sense to expose that in the integration too?

[Odonno]

You'll need to add WithCommand("start", "memory") (I don't think you need the entrypoint too) and then it gets onto the next place where the tests are failing.

I will make the change and see how it goes.

The next place it seems to fail is that it can't resolve an IHttpClientFactory, which looking at the SurrealDbClient type is a required constructor argument.

Yes, it can take an IHttpClientFactory but it is optional (not Required). So, if not provided, it can be nullified and the client can work with the default HttpClient. I don't see why there could be an issue with that. I will dig into it.

Looking at how you can register SurrealDB via their native SDK, I see you can pass a type argument that implements ISurrealDbClient, would it make sense to expose that in the integration too?

Yes, it was there because KeyedService did not exist. I will get rid of this feature because Keyed Service is a better solution. So, no need to change anything for this.

[aaronpowell]

The next place it seems to fail is that it can't resolve an IHttpClientFactory, which looking at the SurrealDbClient type is a required constructor argument.

Yes, it can take an IHttpClientFactory but it is optional (not Required). So, if not provided, it can be nullified and the client can work with the default HttpClient. I don't see why there could be an issue with that. I will dig into it.

I wasn't paying attention properly to the error I was seeing, it is when using the AddKeyedSurrealClient that I see this failure on, and that's because this method uses GetRequestedService<IHttpClientFactory>: https://github.com/surrealdb/surrealdb.net/blob/main/SurrealDb.Net/Extensions/DependencyInjection/ServiceCollectionExtensions.cs#L413-L427

If you don't register keyed, it uses GetService<IHttpClientFactory> which will return a null if the service wasn't registered. Looking at the history, this might just be an oversight, as I see it was changed in surrealdb/surrealdb.net@a7a5a30, but obviously you're more familiar with the Surreal codebase than I am.

[aaronpowell]

Yes, it was there because KeyedService did not exist. I will get rid of this feature because Keyed Service is a better solution. So, no need to change anything for this.

Is it uncommon that someone would create their own type that implements ISurrealDbClient? Or is that more of a legacy approach?

[aaronpowell]

Overall, the code is looking pretty much ready to go, with the obvious exception of the tests needing to be fixed.

[Odonno]

Yes, it was there because KeyedService did not exist. I will get rid of this feature because Keyed Service is a better solution. So, no need to change anything for this.

Is it uncommon that someone would create their own type that implements ISurrealDbClient? Or is that more of a legacy approach?

Legacy approach

[Odonno]

The next place it seems to fail is that it can't resolve an IHttpClientFactory, which looking at the SurrealDbClient type is a required constructor argument.

Yes, it can take an IHttpClientFactory but it is optional (not Required). So, if not provided, it can be nullified and the client can work with the default HttpClient. I don't see why there could be an issue with that. I will dig into it.

I wasn't paying attention properly to the error I was seeing, it is when using the AddKeyedSurrealClient that I see this failure on, and that's because this method uses GetRequestedService<IHttpClientFactory>: https://github.com/surrealdb/surrealdb.net/blob/main/SurrealDb.Net/Extensions/DependencyInjection/ServiceCollectionExtensions.cs#L413-L427

If you don't register keyed, it uses GetService<IHttpClientFactory> which will return a null if the service wasn't registered. Looking at the history, this might just be an oversight, as I see it was changed in surrealdb/surrealdb.net@a7a5a30, but obviously you're more familiar with the Surreal codebase than I am.

I suppose bad copy/paste due to different commit changes. I will fix that.

[Odonno]

I kind of struggle with the conformance tests (non keyed). It fails to read Aspire:Surreal:Client configuration because it expects a string (connection string) and I suppose it gets an Options type.

I think it would be easier/better to only deal with connection string, at least for DB integrations which is the case here. I don't know what is the best practice for this. Can you guide me?

[Odonno]

Ok. I found the issue. I copied misleading code. :( Missing key in the default connection string.

[aaronpowell]

Cool, looks like that's sorted most of the tests and we're just going to wait for surrealdb/surrealdb.net#163 to be resolved to unblock most of the others.