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

♻️ web-server: Refactor users domain for improved layer separation and upgrading to asyncpg #6937

Merged
merged 126 commits into from
Dec 16, 2024

Conversation

pcrespov
Copy link
Member

@pcrespov pcrespov commented Dec 10, 2024

What do these changes do?

ReDoc

The users plugin has quite some old functionality so before adding further logic into it, I decided to do some refactoring to align with the conventions and libraries we should be using.

  • Layer Separation:

    • Clear separation of rest-controller, service, and repository layers within the users domain.
    • The api is now a pure interface for other domains.
    • Sub-domains were renamed for clarity (further refactoring to follow in separate PRs).
  • Schema and Domain Model Separation:

    • Introduced a clear distinction between schemas and domain models.
    • Implemented type adapters for seamless conversion between schemas and domain models:
      • schema.to_model() and schema.from_model().
      • The schema layer is aware of domain models, but domain models remain agnostic of schemas.
  • Repository Migration:

    • Replaced aiopg with asyncpg across all repository code.
    • Addressed shared code (e.g., groups_extra_properties) where necessary, though some duplication exists for now.
  • Misc:

  • User Enums: Moved user-related enums to the common-library for broader reuse.

Related issue/s

How to test

cd services/web/server
make install-dev
pytest -vv --pdb tests/unit/**/test_*user*.py 

Dev-ops

None

@pcrespov pcrespov self-assigned this Dec 10, 2024
@pcrespov pcrespov added a:webserver issue related to the webserver service a:database associated to postgres service and postgres-database package labels Dec 10, 2024
Copy link

codecov bot commented Dec 10, 2024

Codecov Report

Attention: Patch coverage is 88.64353% with 72 lines in your changes missing coverage. Please review.

Project coverage is 87.55%. Comparing base (e391377) to head (56ea4f7).
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6937      +/-   ##
==========================================
- Coverage   88.10%   87.55%   -0.56%     
==========================================
  Files        1592     1603      +11     
  Lines       62354    62680     +326     
  Branches     2013     2069      +56     
==========================================
- Hits        54940    54877      -63     
- Misses       7078     7456     +378     
- Partials      336      347      +11     
Flag Coverage Δ
integrationtests 64.90% <58.27%> (-0.10%) ⬇️
unittests 85.82% <88.01%> (-0.57%) ⬇️
Components Coverage Δ
api 76.84% <ø> (ø)
pkg_aws_library 93.49% <ø> (ø)
pkg_dask_task_models_library 97.09% <ø> (ø)
pkg_models_library 91.36% <90.59%> (+0.11%) ⬆️
pkg_notifications_library 84.57% <ø> (ø)
pkg_postgres_database 88.26% <100.00%> (+0.20%) ⬆️
pkg_service_integration 70.02% <ø> (ø)
pkg_service_library 74.52% <ø> (ø)
pkg_settings_library 90.60% <ø> (ø)
pkg_simcore_sdk 85.38% <ø> (ø)
agent 97.00% <ø> (ø)
api_server 90.13% <ø> (ø)
autoscaling 96.09% <ø> (ø)
catalog 90.57% <ø> (ø)
clusters_keeper 99.48% <ø> (ø)
dask_sidecar 91.26% <ø> (ø)
datcore_adapter 93.18% <ø> (ø)
director 76.40% <ø> (ø)
director_v2 91.40% <ø> (ø)
dynamic_scheduler 97.03% <ø> (ø)
dynamic_sidecar 89.75% <ø> (+0.03%) ⬆️
efs_guardian 90.12% <ø> (ø)
invitations 93.44% <ø> (ø)
osparc_gateway_server ∅ <ø> (∅)
payments 92.66% <ø> (ø)
resource_usage_tracker 89.58% <ø> (ø)
storage 89.54% <ø> (ø)
webclient ∅ <ø> (∅)
webserver 86.16% <86.48%> (-1.58%) ⬇️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e391377...56ea4f7. Read the comment docs.

@pcrespov pcrespov force-pushed the is1779/refactor-users branch from ed2a0d5 to b358fb4 Compare December 10, 2024 22:43
@pcrespov pcrespov added this to the Event Horizon milestone Dec 11, 2024
@pcrespov pcrespov force-pushed the is1779/refactor-users branch from b358fb4 to 8e3bedb Compare December 11, 2024 19:46
Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

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

Very nice. Thanks for addressing my questions.

@pcrespov pcrespov enabled auto-merge (squash) December 16, 2024 13:40
Copy link
Member

@odeimaiz odeimaiz left a comment

Choose a reason for hiding this comment

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

Merci!!

@pcrespov pcrespov disabled auto-merge December 16, 2024 21:05
@pcrespov pcrespov enabled auto-merge (squash) December 16, 2024 21:05
@pcrespov pcrespov disabled auto-merge December 16, 2024 21:08
@pcrespov pcrespov merged commit 1ce9f08 into ITISFoundation:master Dec 16, 2024
90 of 93 checks passed
@pcrespov pcrespov deleted the is1779/refactor-users branch December 17, 2024 07:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:database associated to postgres service and postgres-database package a:models-library a:webserver issue related to the webserver service
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants