Skip to content
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

Enhancing CosmosTemplate to Support Multi-Tenancy at a Container Level #33400

Merged
merged 13 commits into from
Feb 16, 2023
Merged

Enhancing CosmosTemplate to Support Multi-Tenancy at a Container Level #33400

merged 13 commits into from
Feb 16, 2023

Conversation

TheovanKraay
Copy link
Member

@TheovanKraay TheovanKraay commented Feb 8, 2023

Description

This PR adds support for multi-tenancy at a container level via the CosmosFactory. There is a test case added which shows how the user can extend CosmosFactory and use overrideContainerName() in order to dynamically set the container being used by CosmosTemplate and repositories when making calls. Integration test coverage was added for this new feature. Reactive tests are also added for container and database level multitenancy (see PR #32516).

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.

@azure-sdk
Copy link
Collaborator

API change check

APIView has identified API level changes in this PR and created following API reviews.

azure-spring-data-cosmos

* @param cosmosAsyncClient cosmosAsyncClient
* @param databaseName databaseName
*/
public MultiTenantContainerCosmosFactory(CosmosAsyncClient cosmosAsyncClient, String databaseName) {
Copy link
Member

Choose a reason for hiding this comment

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

just curious, why we do not have an constructor accepting containerName? and also in Spring, we do not really use setter pattern?

Copy link
Member Author

@TheovanKraay TheovanKraay Feb 14, 2023

Choose a reason for hiding this comment

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

By default, container name is taken from either entity name, or containerName in @container within entity if defined. So it need not be passed to the constructor by default (but we add the setter/getter here so it can be overridden and called polymorphically in the case of multi-tenant scenario). For databaseName, this has to be passed in, as that has to be defined in config. For getter/setter, yes we should, but these are just sample classes to facilitate the unit tests, and will only be used by them, they are not part of the API (user would create their own, and we'll have examples for that in samples repo).

Copy link
Member

@xinlian12 xinlian12 left a comment

Choose a reason for hiding this comment

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

LGTM, thanks :)

Copy link
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

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

LGTM thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
azure-spring All azure-spring related issues Cosmos
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants