Skip to content
This repository has been archived by the owner on Sep 10, 2024. It is now read-only.

Deduplicate the query bulding logic for all dynamic filters #2979

Merged
merged 11 commits into from
Jul 16, 2024

Conversation

sandhose
Copy link
Member

@sandhose sandhose commented Jul 16, 2024

This can be reviewd commit by commit.

  • Add a trait to help building dynamic filters
  • Use dynamic filters on compatibility sessions
  • Use dynamic filters on compatibility SSO logins
  • Use dynamic filters on OAuth 2.0 sessions
  • Use dynamic filters on upstream OAuth 2.0 links
  • Use dynamic filters on upstream OAuth 2.0 providers
  • Use dynamic filters on user emails
  • Use dynamic filters on users
  • Use dynamic filters on browser sessions
  • Add new filters on the OAuth and compat sessions
  • Use dynamic filters on app sessions by reusing the OAuth/compat sessions filters

This deduplicates the filtering logic for list() and count() methods (as well as finish_bulk() on sessions)

The most special one is the "app sessions" repository, as it does a SELECT UNION on both the OAuth and compat sessions tables, so I did some logic to split an AppSessionFilter into a OAuthSessionFilter and a CompatSessionFilter.
I had to add a few new filters to the OAuthSessionFilter and CompatSessionFilter, but that's it.

I'm fairly confident that most of those filters have associated tests, and most of it was copy-pasting the existing where clauses.

Copy link

cloudflare-workers-and-pages bot commented Jul 16, 2024

Deploying matrix-authentication-service-docs with  Cloudflare Pages  Cloudflare Pages

Latest commit: 1ca82a9
Status: ✅  Deploy successful!
Preview URL: https://bd350bb8.matrix-authentication-service-docs.pages.dev
Branch Preview URL: https://quenting-refactor-dynamic-fi.matrix-authentication-service-docs.pages.dev

View logs

@sandhose sandhose requested a review from reivilibre July 16, 2024 15:59
@sandhose sandhose force-pushed the quenting/refactor-dynamic-filters branch from 420264a to 26ef5c3 Compare July 16, 2024 16:04
@sandhose sandhose force-pushed the quenting/refactor-dynamic-filters branch from 26ef5c3 to 1ca82a9 Compare July 16, 2024 16:08
Copy link
Contributor

@reivilibre reivilibre left a comment

Choose a reason for hiding this comment

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

Thanks for doing that, it's still verbose but at least it's all contained, it feels like a good abstraction to have!

@sandhose sandhose merged commit e89a818 into main Jul 16, 2024
16 checks passed
@sandhose sandhose deleted the quenting/refactor-dynamic-filters branch July 29, 2024 12:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants