Skip to content

Conversation

@giancarloromeo
Copy link
Contributor

@giancarloromeo giancarloromeo commented Sep 12, 2025

What do these changes do?

This PR updates the uniqueness constraint for API keys in the database to include the product name, allowing users to have API keys with the same display name across different products.

  • Updated the api_keys table uniqueness constraint from (display_name, user_id) to (display_name, user_id, product_name)
  • (BONUS) Refactored test fixtures to use a factory pattern for better flexibility
  • Added a new test to verify API keys with the same display name can be created for different products

Related issue/s

How to test

cd services/web/server
make install-dev
pytest -vv --pdb tests/unit/with_dbs/01/test_api_keys.py::test_create_api_keys_same_display_name_different_products

Dev-ops

  • Changed Uniqueness Constraint in api_keys table.

@giancarloromeo giancarloromeo added this to the Cheops milestone Sep 12, 2025
@giancarloromeo giancarloromeo self-assigned this Sep 12, 2025
@giancarloromeo giancarloromeo added the a:webserver webserver's codebase. Assigning the area is particularly useful for bugs label Sep 12, 2025
@giancarloromeo giancarloromeo changed the title πŸ› Update API keys uniqueness constraint πŸ› Update API keys uniqueness constraint (πŸ—ƒοΈ) Sep 12, 2025
@codecov
Copy link

codecov bot commented Sep 12, 2025

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 86.02%. Comparing base (57d948f) to head (da47bbf).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #8363      +/-   ##
==========================================
- Coverage   87.42%   86.02%   -1.40%     
==========================================
  Files        1945     1452     -493     
  Lines       75630    57518   -18112     
  Branches     1321      563     -758     
==========================================
- Hits        66120    49482   -16638     
+ Misses       9126     7902    -1224     
+ Partials      384      134     -250     
Flag Coverage Ξ”
integrationtests 61.02% <ΓΈ> (-2.29%) ⬇️
unittests 86.18% <ΓΈ> (-0.35%) ⬇️
Components Coverage Ξ”
pkg_aws_library βˆ… <ΓΈ> (βˆ…)
pkg_celery_library βˆ… <ΓΈ> (βˆ…)
pkg_dask_task_models_library βˆ… <ΓΈ> (βˆ…)
pkg_models_library 93.09% <ΓΈ> (ΓΈ)
pkg_notifications_library 85.20% <ΓΈ> (ΓΈ)
pkg_postgres_database 87.95% <ΓΈ> (ΓΈ)
pkg_service_integration βˆ… <ΓΈ> (βˆ…)
pkg_service_library βˆ… <ΓΈ> (βˆ…)
pkg_settings_library βˆ… <ΓΈ> (βˆ…)
pkg_simcore_sdk 65.27% <ΓΈ> (ΓΈ)
agent 93.53% <ΓΈ> (ΓΈ)
api_server βˆ… <ΓΈ> (βˆ…)
autoscaling 95.77% <ΓΈ> (ΓΈ)
catalog 92.36% <ΓΈ> (ΓΈ)
clusters_keeper 99.13% <ΓΈ> (ΓΈ)
dask_sidecar 92.37% <ΓΈ> (ΓΈ)
datcore_adapter 97.94% <ΓΈ> (ΓΈ)
director 75.81% <ΓΈ> (-0.18%) ⬇️
director_v2 85.23% <ΓΈ> (-5.74%) ⬇️
dynamic_scheduler 96.27% <ΓΈ> (ΓΈ)
dynamic_sidecar 81.87% <ΓΈ> (-8.59%) ⬇️
efs_guardian 89.62% <ΓΈ> (ΓΈ)
invitations 91.44% <ΓΈ> (ΓΈ)
payments 92.61% <ΓΈ> (ΓΈ)
resource_usage_tracker 92.08% <ΓΈ> (-0.11%) ⬇️
storage βˆ… <ΓΈ> (βˆ…)
webclient βˆ… <ΓΈ> (βˆ…)
webserver 82.37% <ΓΈ> (-5.59%) ⬇️

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 57d948f...da47bbf. Read the comment docs.

πŸš€ New features to boost your workflow:
  • πŸ“¦ JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the uniqueness constraint for API keys in the database to include the product name, allowing users to have API keys with the same display name across different products.

  • Updated the api_keys table uniqueness constraint from (display_name, user_id) to (display_name, user_id, product_name)
  • Refactored test fixtures to use a factory pattern for better flexibility
  • Added a new test to verify API keys with the same display name can be created for different products

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

File Description
packages/postgres-database/src/simcore_postgres_database/models/api_keys.py Updated uniqueness constraint definition to include product_name
packages/postgres-database/src/simcore_postgres_database/migration/versions/7e92447558e0_update_api_keys_uniqueness_constraint.py Database migration to update the constraint
services/web/server/src/simcore_service_webserver/api_keys/_repository.py Updated conflict resolution to include product_name
services/web/server/tests/unit/with_dbs/01/test_api_keys.py Refactored fixtures and added test for same display name across products

@giancarloromeo giancarloromeo added the bug buggy, it does not work as expected label Sep 12, 2025
@giancarloromeo giancarloromeo marked this pull request as ready for review September 12, 2025 12:25
Copy link
Contributor

@wvangeit wvangeit left a comment

Choose a reason for hiding this comment

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

Nice. Thanks.

…com:giancarloromeo/osparc-simcore into is8110/update-api-key-uniqueness-constraint
@mergify
Copy link
Contributor

mergify bot commented Sep 12, 2025

πŸ§ͺ CI Insights

Here's what we observed from your CI run for da47bbf.

βœ… Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries πŸ” CI Insights πŸ“„ Logs
CI system-tests Base branch is broken, but the job passed. Looks like this might be a real fix πŸ’ͺ Broken 0 View View

Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

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

thanks

Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

Thanks!

@giancarloromeo giancarloromeo enabled auto-merge (squash) September 16, 2025 07:06
@giancarloromeo
Copy link
Contributor Author

@Mergifyio queue

@giancarloromeo giancarloromeo added the πŸ€–-automerge marks PR as ready to be merged for Mergify label Sep 16, 2025
@mergify
Copy link
Contributor

mergify bot commented Sep 16, 2025

queue

🟠 Waiting for conditions to match

  • -closed [πŸ“Œ queue requirement]
  • -conflict [πŸ“Œ queue requirement]
  • -draft [πŸ“Œ queue requirement]
  • any of: [πŸ“Œ queue -> configuration change requirements]
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of: [πŸ”€ queue conditions]
    • all of: [πŸ“Œ queue conditions of queue default]
      • #approved-reviews-by >= 2 [πŸ›‘ GitHub branch protection]
      • #approved-reviews-by>=2
      • #changes-requested-reviews-by = 0 [πŸ›‘ GitHub branch protection]
      • #changes-requested-reviews-by=0
      • #review-threads-unresolved = 0 [πŸ›‘ GitHub branch protection]
      • #review-threads-unresolved=0
      • -conflict
      • -draft
      • base=master
      • branch-protection-review-decision = APPROVED [πŸ›‘ GitHub branch protection]
      • label!=πŸ€–-do-not-merge
      • label=πŸ€–-automerge
      • any of: [πŸ›‘ GitHub branch protection]
        • check-skipped = deploy to dockerhub
        • check-neutral = deploy to dockerhub
        • check-success = deploy to dockerhub
      • any of: [πŸ›‘ GitHub branch protection]
        • check-success = system-tests
        • check-neutral = system-tests
        • check-skipped = system-tests
      • any of: [πŸ›‘ GitHub branch protection]
        • check-success = unit-tests
        • check-neutral = unit-tests
        • check-skipped = unit-tests
      • any of: [πŸ›‘ GitHub branch protection]
        • check-success = check OAS' are up to date
        • check-neutral = check OAS' are up to date
        • check-skipped = check OAS' are up to date
      • any of: [πŸ›‘ GitHub branch protection]
        • check-success = integration-tests
        • check-neutral = integration-tests
        • check-skipped = integration-tests
      • any of: [πŸ›‘ GitHub branch protection]
        • check-success = build-test-images (frontend) / build-test-images
        • check-neutral = build-test-images (frontend) / build-test-images
        • check-skipped = build-test-images (frontend) / build-test-images
      • any of: [πŸ›‘ GitHub branch protection]
        • check-success = SonarCloud Code Analysis
        • check-neutral = SonarCloud Code Analysis
        • check-skipped = SonarCloud Code Analysis

@sonarqubecloud
Copy link

@giancarloromeo giancarloromeo merged commit 62565ba into ITISFoundation:master Sep 16, 2025
95 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

πŸ€–-automerge marks PR as ready to be merged for Mergify a:webserver webserver's codebase. Assigning the area is particularly useful for bugs bug buggy, it does not work as expected

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Using the same keyname on the same deployment but different product fails

5 participants