Skip to content

Add support for Cosmos DB and other NoSQL databases #1103

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

ThomasBarnekow
Copy link

As discussed in #1102, this PR adds the NoSqlResourceService<TResource, TId> and related classes and interfaces:

  • NoSqlServiceCollectionExtensions: Used in Startup to add injectables
  • INoSqlQueryLayerComposer: Defines the NoSQL QueryLayer composer contract
  • NoSqlQueryLayerComposer: Implements the NoSQL QueryLayer composer
  • NoSqlHasForeignKeyAttribute: Adds foreign key information to navigation properties
  • NoSqlOwnsManyAttribute: Identifies relationships as owned
  • NoSqlResourceAttribute: Identifies NoSQL resources (as opposed to owned entities)

I will add shareable examples and integration tests in future commits. At the moment, the feature is covered by integration tests of an API that is part of a larger product that we are developing.

The feature supports Cosmos DB out of the box. Microsoft provides an Entity Framework Core provider for Cosmos DB, so the JADNC-provided EntityFrameworkCoreRepository<TResource, TId> can be used with the NoSqlResourceService<TResource, TId>. The latter makes sure that no unsupported queries are made.

I have not yet tested the NoSqlResourceService<TResource, TId> with the MongoRepository<TResource, TId>. We would have to see whether navigation properties are already populated, e.g., when including resources.

The feature is registered by calling the AddNoSqlResourceServices() method as follows:

public class Startup
{
    public void ConfigureServices(IServiceCollection services)
    {
        services.AddNoSqlResourceServices();
    }
}

For each class annotated with the NoSqlResourceAttribute the above extension method registers a NoSqlResourceService<TResource, TId>. In the example below, this would be done for the Author and Article classes. I've used a separate annotation because some resources implementing IIdentifiable<TId> might be owned by other resources and would therefore not have their own controllers and resource services.

The navigation properties of those classes are further annotated with the NoSqlHasForeignKeyAttribute, which provides information on the foreign key property to be used for navigating relationships. While this would not be necessary where an EF Core model exists (e.g., when accessing Cosmos DB using EF Core), such an EF Core model will not exist for arbitrary NoSQL databases.

[NoSqlResource]
public class Author : Identifiable<string>
{
    [Attr]
    public string Name { get; set; } = null!;

    [HasMany]
    [NoSqlHasForeignKey(nameof(Article.AuthorId))]
    public ICollection<Article> Articles { get; set; } = new HashSet<Article>();
}

[NoSqlResource]
public class Article : Identifiable<string>
{
    [Attr]
    public string Title { get; set; } = null!;

    public string AuthorId { get; set; } = null!;

    [HasOne]
    [NoSqlHasForeignKey(nameof(AuthorId))]
    public Author? Author { get; set; }
}

As both the HasManyAttribute and HasOneAttribute are sealed, I used the separate NoSqlHasForeignKeyAttribute. Depending on how deeply you want to integrate this feature, there might be the option to enhance the HasManyAttribute and HasOneAttribute classes to avoid a separate attribute. The same applies to the NoSqlResourceAttribute and the NoSqlOwnsManyAttribute.

This commit adds the NoSqlResourceService and related classes and interfaces:

- NoSqlServiceCollectionExtensions: Used in Startup to add injectables
- INoSqlQueryLayerComposer: Defines the NoSQL QueryLayer composer contract
- NoSqlQueryLayerComposer: Implements the NoSQL QueryLayer composer
- NoSqlHasForeignKeyAttribute: Adds foreign key information to navigation properties
- NoSqlOwnsManyAttribute: Identifies relationships as owned
- NoSqlResourceAttribute: Identifies NoSQL resources (as opposed to owned entities)
This commit disables the following warnings:

- AV2310 (Code block should not contain inline comments)
- AV2318 (Work-tracking TO DO comment should be removed)
- AV2407 (Region should be removed)

It should be allowed to add useful inline comments. In a pull request, there might be some TODOs.
Regions can be useful to structure larger groups of methods, for example.

Further, this commits makes code changes to remove other warnings.
@ThomasBarnekow
Copy link
Author

@bart-degreed, would it be possible to make local builds fail when appveyor builds would also fail, e.g., due to code style-related warnings? I've now had to create multiple commits because I've been told about those build failures in an incremental fashion. In those cases, Visual Studio did not even show any warnings.

@ThomasBarnekow
Copy link
Author

@bart-degreed, to be honest, I am pretty lost right now. Since I could not run the cleanupcode.ps1 script on my local machine, I performed the cleanup manually on each of my files by using ReSharper and your JADNC Full Cleanup profile. In one case, for example, that created trailing spaces in a comment, which I removed. Without those trailing spaces (and other code formatting changes that are seemingly still required), the build failed again.

I've then found out I needed to install PowerShell Core (Windows PowerShell does not suffice) on my machine to run your script. Having run the script (which took ages to complete) and looking at the changes it has made, I could only see that it has reintroduced the trailing spaces in one of my files (NoSqlHasForeignKeyAttribute.cs). It did not change any of my other files for which code formatting changes are seemingly required as per appveyor. However, it made changes to 170 of your code files.

I guess you don't want changes made to 170 of your own code files. And I don't know how to make this work. I'll stop trying for now and wait for your guidance. Unfortunately, this has been very frustrating.

This commit edits files that lead to diff errors on appveyor even after having performed
the ReSharper code cleanup locally (a) using the ReSharper UI, (b) the PowerShell script,
and (c) the exact command line executed on appveyor. Let's hope for the best.
@codecov
Copy link

codecov bot commented Nov 8, 2021

Codecov Report

Merging #1103 (063b04f) into master (b88d39e) will increase coverage by 0.08%.
The diff coverage is 85.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1103      +/-   ##
==========================================
+ Coverage   88.59%   88.67%   +0.08%     
==========================================
  Files         255      270      +15     
  Lines        7127     7684     +557     
==========================================
+ Hits         6314     6814     +500     
- Misses        813      870      +57     
Impacted Files Coverage Δ
...osmosDbExample/Controllers/NonJsonApiController.cs 0.00% <0.00%> (ø)
...osmosDbExample/Controllers/OperationsController.cs 0.00% <0.00%> (ø)
src/Examples/CosmosDbExample/Program.cs 66.66% <66.66%> (ø)
...onApiDotNetCore/Queries/NoSqlQueryLayerComposer.cs 83.76% <83.76%> (ø)
...JsonApiDotNetCore/Services/NoSqlResourceService.cs 84.54% <84.54%> (ø)
.../CosmosDbExample/Definitions/TodoItemDefinition.cs 92.30% <92.30%> (ø)
src/Examples/CosmosDbExample/Startup.cs 96.15% <96.15%> (ø)
...es/CosmosDbExample/Controllers/PeopleController.cs 100.00% <100.00%> (ø)
...CosmosDbExample/Controllers/TodoItemsController.cs 100.00% <100.00%> (ø)
src/Examples/CosmosDbExample/Data/AppDbContext.cs 100.00% <100.00%> (ø)
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b88d39e...063b04f. Read the comment docs.

@ThomasBarnekow
Copy link
Author

@bart-degreed, now that the build works (albeit only with manual code formatting since the code cleanup produces results that are not accepted on appveyor), I would like to talk about testing.

The feature is currently covered by a set of integration tests written for our own API. Since I can't use those here, I could create a sample API and write integration tests for that sample API. Another option would be to reuse some of the integration tests that you have already written. I would need some guidance on what best to do.

Another question would be how and where you run integration tests at the moment. This feature would either require the Cosmos DB Emulator (available for Windows, Docker for Windows, and Docker for Linux) on your local machine or access to Azure Cosmos DB. For the purposes of integration tests, the free tier would definitely be sufficient.

@wayne-o
Copy link
Contributor

wayne-o commented Nov 8, 2021 via email

@bart-degreed
Copy link
Contributor

Hi @ThomasBarnekow, thanks for all the effort you put into this and for sharing it. @maurei and I would like to help you move forward with this. However, speaking for myself, I have some other priorities that I need to work on first.

As we both don't have experience with CosmosDB, it would help if you can provide some getting-started instructions, along with an example project, in order to run this locally and step through the code.

You're asking good questions, for which we don't have the answers yet. We'll need to assess the chosen solution and decide on the scope of this PR, as well as whether it belongs to this or a sibling repo. And how to ensure test coverage. And how to run them in cibuild. And how to package this via NuGet.

You mentioned MongoDB support too, for which we currently use the MongoDB LINQ provider. I found that CosmosDB can be used via the MongoDB API, is that something you considered? I'm curious about the pros and cons, and how well the EF Core provider supports CosmosDB in practice.

We should definitely think through the big picture and where we want to end up eventually, and I'm willing to jump on a call with you at a later time. But for the first PR to get reviewed and merged, I'd like to keep things as simple as possible, then iterate from there. Making things too generic upfront to enable possible future enhancements is the YAGNI anti-pattern that I'd like to avoid, especially because in open-source we never know when contributors lose interest.

@ThomasBarnekow
Copy link
Author

Hi @bart-degreed, I will provide instructions, an example project, and integration tests.

Yes, Cosmos DB supports multiple APIs, including the native SQL API, the MongoDB API, a Gremlin-based Graph API, and others. In practice, you'll have to pick "your" API on a database account level and your choice will depend on where you are coming from. If you have been using MongoDB for other applications, the MongoDB API is likely a good choice. However, Microsoft recommends the native SQL API (which always offers the latest and greatest features first, for example). This is also what was chosen for one of our applications.

I'd say the EF Core provider for Cosmos DB supports Cosmos DB pretty well. There are certain limitations that are due to the database engine itself plus limitations of the EF Core provider as it stands right now. For example, the provider does not support joins and includes right now (see https://docs.microsoft.com/en-us/ef/core/providers/cosmos/limitations). Therefore, without the NoSqlResourceService, you can't really use Cosmos with JADNC. However, I wanted the development team to use JADNC for the API (and somebody else wanted to use Cosmos as a database), so I built the NoSqlResourceService to bridge the two worlds. Since we have only used Azure SQL Database or SQL Server for our previous applications, the Cosmos DB SQL API with the option to use EF Core was the most natural choice for the development team.

Regarding MongoDB support, I don't know exactly what types of queries are supported by your MongoRepository. For example, does it support the inclusion of resources via include=relationshipName? If not, the NoSqlResourceService would add support for that. When changing relationships, though, it would still call MongoRepository methods that are throwing exceptions right now. Depending on the reasons why you did not implement those methods (which are called by the resource service), it might be possible to enhance the resource service to not require those methods to be implemented by the repository.

This commit adds an example for Cosmos DB, using the JsonApiDotNetCoreExample
as the starting point and making changes required for Cosmos and to showcase Cosmos
features (e.g., embedded / owned entities).
Allegedly, Person.Id could not be resolved, which lead to an error on appveyor.
This commit fixes that error by removing the cref.
@ThomasBarnekow
Copy link
Author

The last build failed because the following line of the appsettings.json was deemed too long:

    "DefaultConnection": "AccountEndpoint=https://localhost:8081/;AccountKey=C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw=="

The appveyor build wants it to look like this:

    "DefaultConnection":
      "AccountEndpoint=https://localhost:8081/;AccountKey=C2y6yDjf5/R+ob0N8A7Cgv30VRDJIWEHLM+4QDU5DE2nQ9nDuVTqobD4b8mGGyPMbIZnqyMsEcaGQy67XIw/Jw=="

This is not how you would lay out a JSON file, so this is unexpected.

@ThomasBarnekow
Copy link
Author

Hi @bart-degreed, I have provided an example project (JsonApiDotNetCoreExample.Cosmos) and would write integration tests next. Where do you want me to provide instructions?

@bart-degreed
Copy link
Contributor

For now, the PR description should be fine.

Change the way the bash script is called to try to avoid a runtime error on appveyor.
Use nohup to call a bash script to fix the "Permission denied" issue on appveyor.
This commit makes a temporary change to the Build.ps1 script which needs to be
reverted once the build is successful.
It seems the before_build bash script was not executed.
This all worked on WSL. Why does it not work on appveyor?
This is a temporary thing. Somehow the actual before_build script is not launched
when there is only one image.
@ThomasBarnekow
Copy link
Author

Hi @bart-degreed, it seems to be really hard to make the appveyor build work. All of this works nicely on my local machine, using

  • WSL with Ubuntu and the Azure Cosmos Emulator Docker image for Linux and
  • Windows 10 with the Azure Cosmos Emulator for Windows.

Let's see what happens now. The previous Windows build worked and the integration tests all passed. I've made a change to the Startup class so that in all cases the so-called Direct mode is used to access Cosmos DB. This works on my local machine. However, appveyor might be different.

@ThomasBarnekow
Copy link
Author

@bart-degreed, the integration tests did not work because the connection times out. I don't know why this happens. As I said above, it works on the WSL with Ubuntu on my local machine.

Would it be possible to skip the integration tests on the Ubuntu image and just run them on the Visual Studio 2019 image (where they passed)? Based on what I've read, it is hard to make it work with the Azure Cosmos Emulator for Linux, which is only a pre-release. I've now spent hours trying to make this work and would not want to waste any more time on this.

@ThomasBarnekow
Copy link
Author

ThomasBarnekow commented Nov 15, 2021

Hi @bart-degreed, so here we are. The build works on both Ubuntu and Windows. The only caveat is that I was unable to connect to the Azure Cosmos Emulator for Linux on appveyor (it works on my local Ubuntu on WSL). Therefore, I disabled the CosmosDbTests for the Ubuntu image on appveyor. The CosmosDbTests are run successfully on the Visual Studio 2019 image on appveyor.

To run the example and the integration tests locally, do the following:

Windows

  • Install and launch the Azure Cosmos Emulator for Windows as described in Microsoft's documentation.
  • Run the integration tests.

macOS and Linux

Have a look at Microsoft's documentation for some background on running the emulator on both macOS and Linux.

The following script (located in the solution folder) pulls and runs the Docker image:

# Pull Azure Cosmos Emulator Docker image
echo "Pulling Azure Cosmos Emulator Docker image for Linux ..."
./pull-docker-azure-cosmos-emulator-linux.sh

# Start Azure Cosmos Emulator container
echo "Running Azure Cosmos Emulator Docker container ..."
nohup ./run-docker-azure-cosmos-emulator-linux.sh &

# Wait for Docker container being started in the background
echo "Waiting 60 seconds before trying to download Azure Cosmos Emulator SSL certificate ..."
sleep 60

# Print the background process output to see whether there are any errors
if [ -f "./nohup.out" ]; then
    echo "--- BEGIN CONTENTS OF NOHUP.OUT ---"
    cat ./nohup.out
    echo "--- END CONTENTS OF NOHUP.OUT ---"
fi

Before you can successfully connect to the Azure Cosmos Emulator on macOS or Linux, however, you MUST install the SSL certificate. To do that on macOS, follow the instructions in Microsoft's documentation on how to install the SSL certificate. On Linux, run the following script (also located in the solution folder):

sudo ./install-azure-cosmos-emulator-linux-certificates.sh

All of the above is included in the before_build script in appveyor.yml, which is currently commented out (because we are currently not running the integration tests and don't need the Azure Cosmos Emulator Docker container).

Once you have started the Docker container and installed the SSL certificate, you can run the integration tests.

Set ConnectionMode.Gateway and LimitToEndpoint = true.
Set relatively short RequestTimeout (10 seconds) to fail fast.
@ThomasBarnekow
Copy link
Author

So what do we have here? Some CosmosDbTests fail with a timeout while others pass. I picked a RequestTimeout of 10 seconds, which is shorter than the default (I believe that is 60 seconds) to fail those tests faster if they fail. The Microsoft documentation talks about intermittent failures due to timeouts.

However, since some of the tests pass, the configuration change I've made and tested successfully in a separate repo seems to work here as well. "Here" means "with the Entity Framework Core provider for Cosmos". In my other repo, I used the CosmosClient to check whether it could connect to the Azure Cosmos Emulator both locally (on Ubuntu on WSL) and on appveyor (it works). With the changed configuration, all integration tests also work locally (on Ubuntu on WSL). And as we saw, they "generally" work on appveyor as well. I'll increase the timeout in the next commit and see what happens.

@ThomasBarnekow
Copy link
Author

Isn't that beautiful. All (!) integration tests (including the CosmosDbTests) are passing on both Windows and Linux.

Based on the additional optimization of the Linux build (RunInspectCode and RunCleanupCode are only run on Windows), the total duration for the two jobs was 30 min 39 sec. The duration for the Ubuntu (Linux) image was 16 min 13 sec. The Visual Studio 2019 (Windows) image took 14 min 21 sec to complete. In both cases, 1758 tests were run.

@maurei
Copy link
Member

maurei commented Nov 22, 2021

Thanks for the wait; I've gone through your PR. Thanks for putting all of this together! It's been pretty educative!

Generally speaking, if we were to introduce support for CosmosDb in this repository, ultimately we would need to arrive at a level of implementation that is generic and covers a large selection of features that we currently cover for relational databases, especially those that touch the JSON:API spec, also weighing in if they make sense conceptually in the context of a document database.

I'd say you did a pretty good job of setting up an elaborate experiment demonstrating that JADNC + CosmosDb can work, but the setup is still relatively simple. If we wish to continue the effort of generalizing this, now would be the time to look at the bigger picture to see what should, can and cannot be implemented.

Our integration test codebase provides a good starting point for a structured approach to achieve this. I would be interested in seeing a patched IntegrationTestContext that works for CosmosDb: running all our integration tests against the current state of your effort will teach us a lot about current limitations and ultimately about where we could take the support for CosmosDb.

Just a small selection of thoughts that arise when going through the integration test codebase:

  • I wonder how the support is for filter operators other than equals?
  • Your choice to retrieve relationships in separate calls given that EF Core does not (and might not ever) support Includes for CosmosDb makes sense (generalizing this further will prove an interesting challenge). Because of limited transaction support I foresee a challenge in the support of writing primary together with relationship data when errors occur during such operation.
  • Would we be able to achieve better support for some features if we were to use the CosmosDb SDK directly rather than the EF Core provider?

We're open to getting on a call with you about this to discuss it in more detail.

@ThomasBarnekow
Copy link
Author

Thanks for your review!

Our integration test codebase provides a good starting point for a structured approach to achieve this. I would be interested in seeing a patched IntegrationTestContext that works for CosmosDb: running all our integration tests against the current state of your effort will teach us a lot about current limitations and ultimately about where we could take the support for CosmosDb.

I suggested using your test codebase in an earlier comment. However, I would welcome some guidance on how to best do this.

I wonder how the support is for filter operators other than equals?

While a hypothesis does not replace proper testing, I would expect full support for filter operators and filter expressions that only use "simple" fields (i.e., no field chains involving navigation properties).

Your choice to retrieve relationships in separate calls given that EF Core does not (and might not ever) support Includes for CosmosDb makes sense (generalizing this further will prove an interesting challenge). Because of limited transaction support I foresee a challenge in the support of writing primary together with relationship data when errors occur during such operation.

I believe I have generalized this to a reasonable extent by not making any assumption related to any specific NoSQL database. For example, I used separate (e.g., NoSqlHasForeignKey) annotations rather than inspecting the EF Core Model because other NoSQL databases will not have an EF Core provider.

Are you saying that JSON:API requires transaction support? In other words, if a DBMS does not support transactions, can it not be used for a JSON:API-based RESTful API?

Would we be able to achieve better support for some features if we were to use the CosmosDb SDK directly rather than the EF Core provider?

Hypothesis: No.

The question is which features you would have in mind. So far, I've only seen an advantage of using the SDK directly when writing lots of data in one go. In this case, bulk support will lead to significant performance improvements.

Using the SDK would mean we'd have to write a new IResourceRepository. When using the EF Core provider for Cosmos DB, we can use the EntityFrameworkCoreRepository.

@ThomasBarnekow
Copy link
Author

@maurei, regarding the call, let's do that.

@maurei
Copy link
Member

maurei commented Nov 23, 2021

@maurei, regarding the call, let's do that.

Reach out to me on our gitter channel and we'll set it up.

Are you saying that JSON:API requires transaction support? In other words, if a DBMS does not support transactions, can it not be used for a JSON:API-based RESTful API?

I mentioned it mostly to point out an example of a core feature (as seen from the JSON:API spec) for which there doesn't seem to be a trivial implementation when using a document db. I think it's too early to make a claim as the one you're suggesting, but as per the following bit in the spec,

A request MUST completely succeed or fail (in a single “transaction”). No partial updates are allowed.

this is definitely something we'll need to consider carefully.

I suggested using your test codebase in an earlier comment. However, I would welcome some guidance on how to best do this.

I think duplicating them so that they can be run in parallel tot the current integration tests would be good. The CreateFactory method in IntegrationTestContext is where EF Core is configured to use postgresql, which would be a starting point. I can experiment with this at a later point if you run in with this. A good test suite to start with would be JsonApiDotNetCoreTests.IntegrationTests.ReadWrite.

@maurei
Copy link
Member

maurei commented Dec 2, 2021

Hi @ThomasBarnekow, as promised I'd get back to you with some pointers.

I think it is best to start with the tests in the following namespaces:

  • JsonApiDotNetCoreTests.IntegrationTests.ReadWrite
  • JsonApiDotNetCoreTests.IntegrationTests.QueryStrings

These covers most of te core features of the framework. With these test suites we would learn a great deal already.

I would suggest starting off with just replacing the EF Core database provider in the integration test context mentioned above and see what results from that. I suspect that with such minimal integration, a lot of challenges will already surface. Simultaneously, a lot of tests can just be ignored because at this point no relationships would be supported.

A second step would then be to plug in your NoSqlResourceService and (1) learn about what additional problems this causes for endpoints that have nothing to do with relationships, and (2) get a better idea of where we can take the relationship support going forward.

As a last step: the following cover more advanced use-cases. These are important to learn more about if one can actually accomplish anything more than simple CRUD test cases:

  • JsonApiDotNetCoreTests.IntegrationTests.Archiving and/or .SoftDeletion
  • JsonApiDotNetCoreTests.IntegrationTests.AtomicOperations
  • JsonApiDotNetCoreTests.IntegrationTests.Microservices
  • JsonApiDotNetCoreTests.IntegrationTests.MultiTenancy

@bart-degreed
Copy link
Contributor

Closing, as the work has been abandoned.

Please note that JADNC works out-of-the-box using the built-in EF Core support for CosmosDB (owned entities). This PR attempts to overcome intentional limitations in EF Core, such as using relationships over different collections by sending multiple queries to the database. However, this implementation only works for simple cases and doesn't meet the quality standards (generic solution, full test coverage) that we require to ship as part of JADNC. Another limitation is that the atomic:operations specification requires client-side transaction management, which is not available in CosmosDB.

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

Successfully merging this pull request may close these issues.

4 participants