Fix/ 343 Unify validation messaging (#346)

* fix(api): unify validation messaging

* fix(api): handle typed tab changes

Author: gabrielmeleiro1

app/tests/conftest.py

@@ -4,14 +4,16 @@
 from sqlalchemy import create_engine
 from sqlalchemy.orm import sessionmaker
 from sqlalchemy.pool import StaticPool
-from app.database import Base
-
-os.environ.setdefault("JWT_SECRET", "test-jwt-secret")
-os.environ.setdefault("GITHUB_CLIENT_ID", "test-github-client-id")
-os.environ.setdefault("GITHUB_SECRET", "test-github-secret")
-os.environ.setdefault("GITHUB_REDIRECT", "http://localhost:5173/login/callback")
-os.environ.setdefault("AUTH_DEV_MODE_APPROVE_ALL_LOGINS", "true")
-os.environ.setdefault("COOKIE_SECURE", "false")
+
+os.environ["JWT_SECRET"] = "test-jwt-secret"
+os.environ["GITHUB_CLIENT_ID"] = "test-github-client-id"
+os.environ["GITHUB_SECRET"] = "test-github-secret"
+os.environ["GITHUB_REDIRECT"] = "http://localhost:5173/login/callback"
+os.environ["AUTH_DEV_MODE_APPROVE_ALL_LOGINS"] = "true"
+os.environ["COOKIE_SECURE"] = "false"
+os.environ["APP_BASE_URL"] = "http://localhost:8000"
+
+from app.database import Base  # noqa: E402
 
 
 @pytest.fixture(scope="function")

…sts/datasets/test_upload_url_endpoint.py …/integration/test_upload_url_endpoint.pytests/datasets/test_upload_url_endpoint.py renamed to app/tests/integration/test_upload_url_endpoint.py tests/datasets/test_upload_url_endpoint.py renamed to app/tests/integration/test_upload_url_endpoint.py

@@ -107,6 +107,10 @@ def _clean_scan_result(file_name: str = "data.csv") -> dict:
     }
 
 
+def _app_state(client: TestClient) -> Any:
+    return cast(Any, client.app).state
+
+
 def _downloadable_dataset_object(
     *,
     object_key: str,
@@ -198,7 +202,7 @@ def test_upload_url_creates_pending_dataset_and_returns_presigned_url(
     expected_storage_key = f"datasets/fixed_{filename}"
 
     monkeypatch.setattr(
-        client.app.state.storage,
+        _app_state(client).storage,
         "create_upload_target",
         lambda _filename, **kwargs: UploadTarget(
             storage_key=expected_storage_key,
@@ -281,7 +285,7 @@ def test_local_direct_upload_allows_dataset_owner(
     )
 
     assert response.status_code == 200
-    storage = cast(Any, client.app).state.storage
+    storage = _app_state(client).storage
     assert storage.read_bytes(storage_key) == b"feature,target\n1,0\n"
 
 
@@ -314,7 +318,7 @@ def test_local_direct_upload_rejects_non_owner(client: TestClient, db_session_fa
     )
 
     assert response.status_code == 403
-    storage = cast(Any, client.app).state.storage
+    storage = _app_state(client).storage
     assert storage.object_exists(storage_key) is False
 
 
@@ -344,7 +348,7 @@ def create_upload_target(filename: str, **kwargs):
         )
 
     monkeypatch.setattr(
-        client.app.state.storage,
+        _app_state(client).storage,
         "create_upload_target",
         create_upload_target,
     )
@@ -392,7 +396,7 @@ def create_upload_target(filename: str, **kwargs):
         )
 
     monkeypatch.setattr(
-        client.app.state.storage,
+        _app_state(client).storage,
         "create_upload_target",
         create_upload_target,
     )
@@ -433,7 +437,7 @@ def test_upload_url_allows_same_dataset_name_with_different_checksum(
         db.commit()
 
     monkeypatch.setattr(
-        client.app.state.storage,
+        _app_state(client).storage,
         "create_upload_target",
         lambda filename, **kwargs: UploadTarget(
             storage_key=f"datasets/fixed_{filename}",
@@ -480,7 +484,7 @@ def test_upload_url_allows_duplicate_dataset_name_for_different_owner(
         db.commit()
 
     monkeypatch.setattr(
-        client.app.state.storage,
+        _app_state(client).storage,
         "create_upload_target",
         lambda filename, **kwargs: UploadTarget(
             storage_key=f"datasets/fixed_{filename}",
@@ -523,7 +527,7 @@ def test_confirm_upload_triggers_scan_and_returns_202(
             )
         )
         db.commit()
-    client.app.state.storage.write_bytes(
+    _app_state(client).storage.write_bytes(
         "datasets/generated_data.csv", b"feature,target\n1,0\n"
     )
 
@@ -598,7 +602,7 @@ def test_confirm_upload_marks_dataset_objects_uploaded(
         )
         db.commit()
 
-    client.app.state.storage.write_bytes(storage_key, b"a,b\n1,2\n")
+    _app_state(client).storage.write_bytes(storage_key, b"a,b\n1,2\n")
     scan_calls = []
 
     def fake_scan(**kwargs):
@@ -788,10 +792,10 @@ def test_download_multiple_dataset_objects_preserves_directory_structure(
         )
         db.commit()
 
-    client.app.state.storage.write_bytes(
+    _app_state(client).storage.write_bytes(
         final_storage_keys[0], b"split,value\ntrain,1\n"
     )
-    client.app.state.storage.write_bytes(
+    _app_state(client).storage.write_bytes(
         final_storage_keys[1], b"split,value\ntest,2\n"
     )
 
@@ -814,13 +818,13 @@ def test_upload_url_returns_direct_s3_upload_contract(
     uploader_id = uuid.uuid4()
     access_token = _create_access_token_for_user(db_session_factory, uploader_id)
     storage = _FakeS3Storage()
-    monkeypatch.setattr(client.app.state, "storage", storage)
+    monkeypatch.setattr(_app_state(client), "storage", storage)
     monkeypatch.setattr(
-        client.app.state,
+        _app_state(client),
         "settings",
         replace(
-            client.app.state.settings,
-            upload=replace(client.app.state.settings.upload, expires_seconds=120),
+            _app_state(client).settings,
+            upload=replace(_app_state(client).settings.upload, expires_seconds=120),
         ),
     )
 
@@ -872,7 +876,7 @@ def test_upload_url_marks_large_s3_upload_contract_as_multipart(
     uploader_id = uuid.uuid4()
     access_token = _create_access_token_for_user(db_session_factory, uploader_id)
     storage = _FakeS3Storage()
-    monkeypatch.setattr(client.app.state, "storage", storage)
+    monkeypatch.setattr(_app_state(client), "storage", storage)
 
     response = client.post(
         "/api/datasets/upload-url",
@@ -897,7 +901,7 @@ def test_upload_url_persists_directory_structure_metadata_from_description(
     access_token = _create_access_token_for_user(db_session_factory, uploader_id)
 
     monkeypatch.setattr(
-        client.app.state.storage,
+        _app_state(client).storage,
         "create_upload_target",
         lambda filename, **kwargs: UploadTarget(
             storage_key=f"datasets/batch/{Path(filename).name}",
@@ -981,7 +985,7 @@ def test_confirm_upload_verifies_s3_object_and_blocks_scan_on_failure(
     uploader_id = uuid.uuid4()
     access_token = _create_access_token_for_user(db_session_factory, uploader_id)
     storage = _FakeS3Storage()
-    monkeypatch.setattr(client.app.state, "storage", storage)
+    monkeypatch.setattr(_app_state(client), "storage", storage)
 
     create_response = client.post(
         "/api/datasets/upload-url",
@@ -1054,7 +1058,7 @@ def test_upload_url_accepts_unsupported_format_and_persists_content_type(
     expected_storage_key = f"datasets/fixed_{filename}"
 
     monkeypatch.setattr(
-        client.app.state.storage,
+        _app_state(client).storage,
         "create_upload_target",
         lambda _filename, **kwargs: UploadTarget(
             storage_key=expected_storage_key,
@@ -1231,7 +1235,7 @@ def fake_create_upload_target(filename: str, prefix: str | None = None):
         )
 
     monkeypatch.setattr(
-        client.app.state.storage, "create_upload_target", fake_create_upload_target
+        _app_state(client).storage, "create_upload_target", fake_create_upload_target
     )
 
     response = client.post(

app/tests/unit/storage/test_config_and_factory.py

@@ -12,6 +12,9 @@ def test_settings_defaults(monkeypatch):
     monkeypatch.delenv("CLAMD_HOST", raising=False)
     monkeypatch.delenv("CLAMD_PORT", raising=False)
     monkeypatch.delenv("CLAMD_TIMEOUT_SECONDS", raising=False)
+    monkeypatch.delenv("UPLOAD_TARGET", raising=False)
+    monkeypatch.delenv("UPLOAD_LOCATION", raising=False)
+    monkeypatch.delenv("UPLOAD_URL_EXPIRES_SECONDS", raising=False)
     monkeypatch.delenv("COOKIE_SECURE", raising=False)
     monkeypatch.delenv("APP_BASE_URL", raising=False)
 
@@ -28,6 +31,9 @@ def test_settings_defaults(monkeypatch):
     assert settings.github_issues.owner == "koevoet1221"
     assert settings.github_issues.repo == "openmlupload-testing"
     assert settings.app_base_url == "http://localhost:8000"
+    assert settings.upload.target == "uploads"
+    assert settings.upload.location == "default"
+    assert settings.upload.expires_seconds == 3600
 
 
 def test_cookie_secure_can_be_disabled_for_local_http(monkeypatch):
@@ -46,6 +52,25 @@ def test_app_base_url_can_be_overridden(monkeypatch):
     assert settings.app_base_url == "https://upload.example.com"
 
 
+def test_upload_settings_can_be_overridden(monkeypatch):
+    monkeypatch.setenv("UPLOAD_TARGET", "team-uploads")
+    monkeypatch.setenv("UPLOAD_LOCATION", "eu")
+    monkeypatch.setenv("UPLOAD_URL_EXPIRES_SECONDS", "7200")
+
+    settings = Settings.from_env()
+
+    assert settings.upload.target == "team-uploads"
+    assert settings.upload.location == "eu"
+    assert settings.upload.expires_seconds == 7200
+
+
+def test_invalid_presigned_url_expiry_raises(monkeypatch):
+    monkeypatch.setenv("UPLOAD_URL_EXPIRES_SECONDS", "0")
+
+    with pytest.raises(ValueError):
+        Settings.from_env()
+
+
 def test_invalid_backend_raises(monkeypatch):
     monkeypatch.setenv("STORAGE_BACKEND", "dropbox")