search

citusdata / django-multitenant

Python/Django support for distributed multi-tenant databases like Postgres+Citus
MIT License
730 stars 118 forks source link

Adds DRF support #157

Closed gurkanindibay closed 1 year ago

gurkanindibay commented 1 year ago

In response to community requests, this PR adds support for using Django-Multitenant with DjangoRestFramework. The documentation for the current implementation of django-multitenant said that any Django app could become multi-tenant by simply adding a middleware. However, I found that when utilizing viewsets, all records are fetched in the first request before logging in, independent of the current tenant. I developed a base viewset and corresponding tests to fix this problem and prevent leaks. While django-restframework is not required to be utilized with django-multitenant, I also included a check to produce a sensible error in environments without it. In order to reflect these changes, the documentation has also been revised.

codecov[bot] commented 1 year ago

Codecov Report

Merging #157 (76f6fe2) into main (38e0970) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #157   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           36        43    +7     
  Lines         1111      1180   +69     
=========================================
+ Hits          1111      1180   +69
Impacted Files Coverage Δ
..._alter_account_id_alter_aliasedtask_id_and_more.py 100.00% <100.00%> (ø)
django_multitenant/tests/models.py 100.00% <100.00%> (ø)
django_multitenant/tests/serializers.py 100.00% <100.00%> (ø)
django_multitenant/tests/settings.py 100.00% <100.00%> (ø)
django_multitenant/tests/test_migrations.py 100.00% <100.00%> (ø)
django_multitenant/tests/test_missing_modules.py 100.00% <100.00%> (ø)
django_multitenant/tests/test_viewsets.py 100.00% <100.00%> (ø)
django_multitenant/tests/urls.py 100.00% <100.00%> (ø)
django_multitenant/tests/utils.py 100.00% <100.00%> (ø)
django_multitenant/tests/views.py 100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

kardeepak77 commented 5 months ago

django_multitenant.middlewares.MultitenantMiddleware isn't calling unset_current_tenant() Am I missing anything?

The unsetting of the tenant is essential because of how webservers work Since the tenant is set as a thread local, the thread is not killed after the request is processed So after processing of the request, we need to ensure that the tenant is unset Especially required if you have public users accessing the site

 This is also essential if you have admin users not related to a tenant.