Skip to content

Commit

Permalink
PENG-2152 Coerced empty identifiers to None values
Browse files Browse the repository at this point in the history
Added validators to make sure that the value for `identifier` is coerced
to `None` if the string is empty.

Also added a validator to make sure that the `name` field cannot be
empty.

Added unit tests for the added logic.
  • Loading branch information
dusktreader committed Apr 10, 2024
1 parent 32b9c7b commit 471bf88
Show file tree
Hide file tree
Showing 4 changed files with 103 additions and 1 deletion.
2 changes: 2 additions & 0 deletions jobbergate-api/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ This file keeps track of all notable changes to jobbergate-api

## Unreleased

- Added logic to coerce empty `identifier` on job script templates to a `None` value [PENG-2152]
- Added logic disallow empty `name` on job script templates [PENG-2152]

## 5.0.0a1 -- 2024-04-04
## 5.0.0a0 -- 2024-03-26
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from typing import Any, Optional

import pydantic
from pydantic import BaseModel
from pydantic import BaseModel, validator

from jobbergate_api.apps.constants import FileType
from jobbergate_api.apps.schemas import LengthLimitedStr, TableResource
Expand Down Expand Up @@ -116,6 +116,24 @@ class JobTemplateCreateRequest(BaseModel):
description: LengthLimitedStr | None
template_vars: dict[LengthLimitedStr, Any] | None

@validator("name")
def not_empty_str(cls, value):
"""
Do not allow a string value to be empty.
"""
if value == "":
raise ValueError("Cannot be an empty string")
return value

@validator("identifier")
def empty_str_to_none(cls, value):
"""
Coerce an empty string value to None.
"""
if value == "":
return None
return value

class Config:
schema_extra = job_template_meta_mapper

Expand All @@ -128,6 +146,24 @@ class JobTemplateCloneRequest(BaseModel):
description: LengthLimitedStr | None
template_vars: dict[LengthLimitedStr, Any] | None

@validator("name")
def not_empty_str(cls, value):
"""
Do not allow a string value to be empty.
"""
if value == "":
raise ValueError("Cannot be an empty string")
return value

@validator("identifier")
def empty_str_to_none(cls, value):
"""
Coerce an empty string value to None.
"""
if value == "":
return None
return value

class Config:
schema_extra = job_template_meta_mapper

Expand All @@ -141,6 +177,24 @@ class JobTemplateUpdateRequest(BaseModel):
template_vars: dict[LengthLimitedStr, Any] | None
is_archived: bool | None

@validator("name")
def not_empty_str(cls, value):
"""
Do not allow a string value to be empty.
"""
if value == "":
raise ValueError("Cannot be an empty string")
return value

@validator("identifier")
def empty_str_to_none(cls, value):
"""
Coerce an empty string value to None.
"""
if value == "":
return None
return value

class Config:
schema_extra = job_template_meta_mapper

Expand Down
45 changes: 45 additions & 0 deletions jobbergate-api/tests/apps/job_script_templates/test_routers.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,51 @@ async def test_create_job_template__success(
assert response_data["template_vars"] == dict(foo="bar")


async def test_create_job_template__fails_if_name_is_empty(
client,
fill_job_template_data,
inject_security_header,
synth_services,
):
payload = fill_job_template_data(
identifier="create-template",
name="",
description="This is a test template",
template_vars=dict(foo="bar"),
)

tester_email = payload.pop("owner_email")
inject_security_header(tester_email, Permissions.JOB_TEMPLATES_EDIT)

response = await client.post("jobbergate/job-script-templates", json=payload)
assert response.status_code == 422
assert "Cannot be an empty string" in response.text
assert (await synth_services.crud.template.count()) == 0


async def test_create_job_template__coerces_empty_identifier_to_None(
client,
fill_job_template_data,
inject_security_header,
synth_services,
):
payload = fill_job_template_data(
identifier="",
name="Test Template",
description="This is a test template",
template_vars=dict(foo="bar"),
)

tester_email = payload.pop("owner_email")
inject_security_header(tester_email, Permissions.JOB_TEMPLATES_EDIT)

response = await client.post("jobbergate/job-script-templates", json=payload)
assert response.status_code == 201, f"Create failed: {response.text}"
response_data = response.json()

assert response_data["identifier"] == None


async def test_create_job_template__fail_unauthorized(client, fill_job_template_data, synth_services):
"""Test that the job template creation fails if the user is unauthorized."""
payload = fill_job_template_data()
Expand Down
1 change: 1 addition & 0 deletions jobbergate-api/tests/apps/job_submissions/test_schemas.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import pytest

from jobbergate_api.apps.job_submissions.schemas import JobSubmissionCreateRequest, JobSubmissionUpdateRequest


Expand Down

0 comments on commit 471bf88

Please sign in to comment.