Reduce the number of SQL queries in updates of groups

[stephane]

After having read the PR #230, my initial intention was to focus on SQL queries. I think it's more efficient to reduce the number of SQL queries to speed up the group creation (round-trip latency could be significant with DB). So my initial patch leveraged create_bulk of Django ORM but I found out about the signal issues in #220 and I changed my mind to provide something less intrusive.

I'll be happy to provide a create_bulk version if you want.

Changes:

• only one query instead of one for each group when no MIRROR_GROUPS
• the use of iterator() inside a list() creates a useless SQL cursor
• avoid to create Django instances of groups by using values_list()
• replace a frozenset by a tuple
• add test for MIRROR_GROUPS

I had to disable Black in my editor to not mess the code ;)

[codecov[bot]]

Codecov Report

Merging #255 (e440441) into master (de92fa1) will increase coverage by 0.8%. The diff coverage is 100.0%.

@@           Coverage Diff            @@
##           master    #255     +/-   ##
========================================
+ Coverage    85.4%   86.3%   +0.8%     
========================================
  Files          11       8      -3     
  Lines         556     497     -59     
========================================
- Hits          475     429     -46     
+ Misses         81      68     -13     

Impacted Files	Coverage Δ
django_auth_adfs/backend.py	86.4% <100.0%> (+2.3%)	:arrow_up:
django_auth_adfs/views.py
django_auth_adfs/__init__.py
django_auth_adfs/urls.py

[JonasKs]

Hi! Thank you so much for this. It all seems valid to me, and code looks good.

I’d like @tim-schilling to say his opinion before I merge though 😊

[stephane]

Strange crash of GithHub Actions: /home/runner/work/_temp/13a6f92d-c22a-414f-bd68-e59415335718.sh: /home/runner/work/django-auth-adfs/django-auth-adfs/.venv/bin/pip: /home/runner/work/django-auth-adfs/django-auth-adfs/.venv/bin/python: bad interpreter: No such file or directory

Let's cool down the servers before testing again... ;)

[stephane]

I don't find the button to re-run jobs BTW...

[JonasKs]

Trying to re-run pipeline, but GitHub is giving me errors.. I’ll try later again.

[JonasKs]

Pipelines seem to have completed now 😊

[JonasKs]

@tim-schilling , If you'd like to look over again? 😊

[JonasKs]

Thank you, @stephane! 😊

I'll get this released after work today.

[JonasKs]

Released! 😊

https://github.com/snok/django-auth-adfs/releases/tag/1.11.1

[stephane]

Thank you all for the review and the release.