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

Should ITenantInfo have connection string removed? #624

Closed
PeterKneale opened this issue Dec 27, 2022 · 3 comments
Closed

Should ITenantInfo have connection string removed? #624

PeterKneale opened this issue Dec 27, 2022 · 3 comments

Comments

@PeterKneale
Copy link

  • I feel like given there is support for a custom implementation of the ITenantInfo this property should be removed / deprecated and users of the library can implement it upon their own class should they require it.

Rationale:

  • There may be no tenant specific connection strings involved (shared schema pattern with row level security etc)
  • There can be multiple connection strings involved, per tenant elastic search, per tenant redis, multiple relational stores etc .
  • It feels like an implementation detail that the library shouldnt concern itself with.
    public interface ITenantInfo
    {
        string? Id { get; set; }
        string? Identifier { get; set;  }
        string? Name { get; set; }
        string? ConnectionString { get; set; } <--
    }
@PeterKneale PeterKneale changed the title Should ITenantInfo has connection string removed? Should ITenantInfo have connection string removed? Dec 27, 2022
@natelaff
Copy link
Contributor

I've brought this up before as well. #325

@AndrewTriesToCode
Copy link
Sponsor Contributor

Yep I agree with both of you. It's be a breaking change and I'm trying to group it in with some others for a major version bump.

AndrewTriesToCode added a commit that referenced this issue Apr 21, 2024
BREAKING CHANGE: Connection string is removed because in many cases it is not needed. Closes #624
github-actions bot pushed a commit that referenced this issue Apr 21, 2024
## [7.0.0](v6.13.1...v7.0.0) (2024-04-21)

### ⚠ BREAKING CHANGES

* Many namespaces have been updated for consistency. Most code will only need to reference the `Finbuckle.MultiTenant` namespace.
* Connection string is removed because in many cases it is not needed. Closes #624
* all unique indexes and the UserLogin primary key in the standard Identity models are adjusted to include the tenant id
* (I)MultiTenantContext and (I)TenantInfo are no longer available via DI. Use IMultiTenantContextAccessor instead. Also IMultiTenantContext nullability reworked and should never be null.
* WithPerTenantOptions replaced by ConfigurePerTenant service collection extensions methods.

Added support for `OptionsBuilder` API and more efficient per-tenant options overall.

### Features

* better options support ([#681](#681)) ([1859017](1859017))
* change default MultiTenantIdentityDbContext default index and key behavior ([81f5612](81f5612))
* MultiTenantDbContext and MultiTenantIdentityDbContext support for IMultiTenantContextAccessor DI ([9015085](9015085))
* namespace cleaned up ([b354838](b354838))
* refactor DI and improve nullability ([eca24bf](eca24bf))
* remove ConnectionString from ITenantInfo and TenantInfo ([f4e20db](f4e20db))

### Bug Fixes

* AdjustKey correctly adding TenantId to primary and foreign keys ([613b4a8](613b4a8))
@AndrewTriesToCode
Copy link
Sponsor Contributor

🎉 This issue has been resolved in version 7.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

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

No branches or pull requests

3 participants