TSPS-357 Add admin endpoint to get a user's quota and update their quota #162

jsotobroad · 2024-11-18T21:49:27Z

Description

At some point we may want to increase a users quota above the default. This admin endpoint makes it possible for us to do so if we want to.

Jira Ticket

https://broadworkbench.atlassian.net/browse/TSPS-357

mmorgantaylor

looking good, some questions and suggestions

mmorgantaylor · 2024-11-19T20:50:58Z

common/openapi.yml

@@ -48,6 +48,51 @@ paths:
        500:
          $ref: '#/components/responses/ServerError'

+  /api/admin/v1/quota/{pipelineName}/{userId}:


nit: for the user-facing endpoint we call it quotas, can we be consistent here?

Suggested change

/api/admin/v1/quota/{pipelineName}/{userId}:

/api/admin/v1/quotas/{pipelineName}/{userId}:

yea i was following the pattern we made for the admin pipeline path, should i update that to pipelines as well?

(i like the idea of it matching with our user facing apis)

yeah thanks for fixing pipelines too!

mmorgantaylor · 2024-11-19T20:54:24Z

common/openapi.yml

@@ -859,6 +927,15 @@ components:
        wdlMethodVersion:
          $ref: "#/components/schemas/PipelineWdlMethodVersion"

+    UpdateQuotaLimitRequestBody:
+      description: |
+        json object containing the admin provided information to update a users quota limit with


nit wording

Suggested change

json object containing the admin provided information to update a users quota limit with

json object containing the admin provided information used to update a user's quota limit

mmorgantaylor · 2024-11-19T20:54:55Z

common/openapi.yml

+    UserId:
+      name: userId
+      in: path
+      description: A string identifier to used to identify a terra user


Suggested change

description: A string identifier to used to identify a terra user

description: A string identifier to used to identify a Terra user

mmorgantaylor · 2024-11-19T20:55:16Z

common/openapi.yml

+        500:
+          $ref: '#/components/responses/ServerError'
+    patch:
+      summary: Update attributes for a given pipeline.


update this summary

mmorgantaylor · 2024-11-19T20:55:39Z

common/openapi.yml

+      - $ref: '#/components/parameters/PipelineName'
+      - $ref: '#/components/parameters/UserId'
+    get:
+      summary: Get description for a given pipeline.


update this summary

Suggested change

summary: Get description for a given pipeline.

summary: Get a user's current quota limit for a given pipeline.

mmorgantaylor · 2024-11-19T20:58:09Z

service/src/main/java/bio/terra/pipelines/app/controller/AdminApiController.java

@@ -74,6 +91,28 @@ public ResponseEntity<ApiAdminPipeline> updatePipeline(
    return new ResponseEntity<>(pipelineToApiAdminPipeline(updatedPipeline), HttpStatus.OK);
  }

+  @Override
+  public ResponseEntity<ApiAdminQuota> updateQuotaLimit(


similarly could this be updateQuotaLimitForUserAndPipeline or similar?

mmorgantaylor · 2024-11-19T21:00:24Z

service/src/main/java/bio/terra/pipelines/service/QuotasService.java

+   * @param newQuotaLimit - the new quota limit
+   * @return - the updated user quota
+   */
+  public UserQuota updateQuotaLimit(UserQuota userQuota, int newQuotaLimit) {


for clarity could we call this adminUpdateUserQuotaLimit ? thank you for mentioning that this is meant to be called from the admin controller.

mmorgantaylor · 2024-11-19T21:02:01Z

service/src/main/java/bio/terra/pipelines/service/QuotasService.java

+   * @param pipelineName - the pipeline name
+   * @return - true if the user quota is present, false otherwise
+   */
+  public boolean isUserQuotaPresent(String userId, PipelinesEnum pipelineName) {


since this doesn't do any other authz checking, can we mark this as also must be called by an admin controller? (just in documentation / function name, as above)

i would say this could be called from wherever cuz its not actually altering the database. The other function is altering the database in a way we dont expect the service to be doing

that's a good point, sounds good 👍

mmorgantaylor · 2024-11-19T21:15:59Z

service/src/main/java/bio/terra/pipelines/app/controller/AdminApiController.java

+
+    // Check if a row exists for this user + pipeline. Throw an error if it doesn't
+    // A row should exist if a user has run into a quota issue before.
+    if (!quotasService.isUserQuotaPresent(userId, validatedPipelineName)) {


curious about the thought process behind this approach, versus adding an option to getQuotaForUserAndPipeline that specifies whether to create a new row if the user isn't found and just reusing that method below?

hmmm well calling the other admin endpoint will actually create that row for the user cuz of how thegetQuotaForUserAndPipeline function works so this is all kind of moot. What scenario do we actually want to catch here? Is it that the user has used our service before? is it that the user_id is an actual terra user_id?

we could write a get function that errors out if the row doesnt exist instead of generating a new one for these admin calls

yeah I'm saying we could add an input to getQuotaForUserAndPipeline that's like addRowIfAbsent=True and toggle that functionality so we could reuse it, if that input is False and there's no row for that user then we throw. if that feels too complicated, that's fine, just an idea that I'm not at all tied to

mmorgantaylor · 2024-11-19T21:17:38Z

service/src/main/java/bio/terra/pipelines/service/QuotasService.java

+   * @return - the updated user quota
+   */
+  public UserQuota updateQuotaLimit(UserQuota userQuota, int newQuotaLimit) {
+    if (newQuotaLimit < userQuota.getQuotaConsumed()) {


so we would allow setting the newQuotaLimit to be equal to the user's current QuotaConsumed, thereby preventing any further activity, is that right? (sounds good to me)

mmorgantaylor · 2024-11-20T14:19:55Z

common/openapi.yml

@@ -4,7 +4,7 @@ info:
  version: 1.0.0
 paths:
  # Admin APIs
-  /api/admin/v1/pipeline/{pipelineName}:
+  /api/admin/v1/pipelines/{pipelineName}:


ah nice catch, thanks!

mmorgantaylor

looks good to me, if you want to refactor the quotasService.getQuotaForUserAndPipeline to add an option to error if the user doesn't exist, cool, but I'm good with what we have here.

sonarcloud · 2024-11-20T17:48:25Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

mmorgantaylor

love the updates!

Jose Soto added 2 commits November 18, 2024 15:05

first stopping point

9bf2323

test and logic working - first pass

6b57c70

jsotobroad requested a review from mmorgantaylor November 18, 2024 21:49

Jose Soto added 2 commits November 18, 2024 16:59

copy paste is hard

3a572f8

really hard...

1852d28

mmorgantaylor reviewed Nov 19, 2024

View reviewed changes

first pass at PR comments!

27a9bdc

mmorgantaylor reviewed Nov 20, 2024

View reviewed changes

mmorgantaylor approved these changes Nov 20, 2024

View reviewed changes

Jose Soto added 3 commits November 20, 2024 12:10

PR feedback, new function to return optional UserQuota

ce11586

add test for admin controller scenario

b6d7199

spotlessss

0636274

mmorgantaylor approved these changes Nov 20, 2024

View reviewed changes

jsotobroad merged commit 04ac01d into main Nov 20, 2024
14 checks passed

jsotobroad deleted the js_TSPS-357 branch November 20, 2024 19:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TSPS-357 Add admin endpoint to get a user's quota and update their quota #162

TSPS-357 Add admin endpoint to get a user's quota and update their quota #162

jsotobroad commented Nov 18, 2024

mmorgantaylor left a comment

mmorgantaylor Nov 19, 2024

jsotobroad Nov 19, 2024 •

edited

Loading

mmorgantaylor Nov 20, 2024

mmorgantaylor Nov 19, 2024

mmorgantaylor Nov 19, 2024

mmorgantaylor Nov 19, 2024

mmorgantaylor Nov 19, 2024

mmorgantaylor Nov 19, 2024

mmorgantaylor Nov 19, 2024

mmorgantaylor Nov 19, 2024

jsotobroad Nov 19, 2024 •

edited

Loading

mmorgantaylor Nov 20, 2024

mmorgantaylor Nov 19, 2024

jsotobroad Nov 19, 2024

mmorgantaylor Nov 20, 2024

mmorgantaylor Nov 19, 2024

jsotobroad Nov 19, 2024

mmorgantaylor Nov 20, 2024

mmorgantaylor left a comment

sonarcloud bot commented Nov 20, 2024

mmorgantaylor left a comment

	/api/admin/v1/quota/{pipelineName}/{userId}:
	/api/admin/v1/quotas/{pipelineName}/{userId}:

	json object containing the admin provided information to update a users quota limit with
	json object containing the admin provided information used to update a user's quota limit

	description: A string identifier to used to identify a terra user
	description: A string identifier to used to identify a Terra user

	summary: Get description for a given pipeline.
	summary: Get a user's current quota limit for a given pipeline.

TSPS-357 Add admin endpoint to get a user's quota and update their quota #162

TSPS-357 Add admin endpoint to get a user's quota and update their quota #162

Conversation

jsotobroad commented Nov 18, 2024

Description

Jira Ticket

mmorgantaylor left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsotobroad Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jsotobroad Nov 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mmorgantaylor left a comment

Choose a reason for hiding this comment

sonarcloud bot commented Nov 20, 2024

Quality Gate passed

mmorgantaylor left a comment

Choose a reason for hiding this comment

jsotobroad Nov 19, 2024 •

edited

Loading

jsotobroad Nov 19, 2024 •

edited

Loading