From 1effe6f41ccdc08ace81929c2e64bdac76a12b05 Mon Sep 17 00:00:00 2001 From: yoshifuminakamura Date: Mon, 14 Sep 2026 14:23:50 +0900 Subject: [PATCH] Consolidate measurement artifact contracts Signed-off-by: yoshifuminakamura --- docs/guides/add-app.md | 3 +- docs/guides/developer-reference.md | 5 +- result_server/routes/api.py | 77 +++---------- result_server/routes/results_detail_routes.py | 69 +++-------- result_server/tests/test_api_routes.py | 14 +++ .../tests/test_measurement_artifacts.py | 107 ++++++++++++++++++ result_server/utils/evidence_packet.py | 23 ++-- result_server/utils/measurement_artifacts.py | 102 +++++++++++++++++ result_server/utils/public_reuse.py | 25 ++-- result_server/utils/result_detail_view.py | 50 +++----- result_server/utils/result_file.py | 16 +-- 11 files changed, 305 insertions(+), 186 deletions(-) create mode 100644 result_server/tests/test_measurement_artifacts.py create mode 100644 result_server/utils/measurement_artifacts.py diff --git a/docs/guides/add-app.md b/docs/guides/add-app.md index b159221..3e6f79d 100644 --- a/docs/guides/add-app.md +++ b/docs/guides/add-app.md @@ -487,7 +487,8 @@ tar -czf ../results/padata0.tgz ./pa Fugaku 系アプリでは、アプリ側が profiler tool を内部で選び、Benchkit 共通の `bk_profiler` helper に渡す形が扱いやすいです。 `bk_profiler` は profiler ごとの raw data / postprocess report をまとめて `results/padata*.tgz` に保存し、archive 内の `bk_profiler_artifact/meta.json` に metadata を入れます。Benchkit や推定 package はこの `meta.json` を見て、tool、level、report kind を機械的に判断できます。 -`timing_observations` が `results/*.json` を参照する場合も、Result 送信時に同じ Measurement Artifacts として保存されます。 + +アプリが独自の詳細 timer table を持つ場合は、まず小さな `results/*.json` として保存し、`bk_record_timing_observation` で登録してください。この JSON は Result 送信時に Measurement Artifacts として保存されます。`timing_observations` は未レビューの観測値を残すための任意機能であり、`SECTION:` / `OVERLAP:` や `fom_breakdown` へ昇格するには、timer ID、inclusive / exclusive の扱い、overlap window の意味を別途レビューします。 `fapp` では共通 level として次を扱います。 diff --git a/docs/guides/developer-reference.md b/docs/guides/developer-reference.md index bf7d96e..5cb6672 100644 --- a/docs/guides/developer-reference.md +++ b/docs/guides/developer-reference.md @@ -283,5 +283,6 @@ Treat missing `source_info`, `fom_breakdown`, or artifact references as follow-u Detailed timing artifacts may be recorded through `timing_observations` before they are promoted to `fom_breakdown`; do not treat every detailed timer or profiler region as an additive estimation section without an app-specific -mapping review. Referenced `results/*.json` timing files and profiler archives -are uploaded as Measurement Artifacts by the result sender. +mapping review. Apps should use `bk_record_timing_observation` for this +handoff. Referenced `results/*.json` timing files and profiler archives are +uploaded as Measurement Artifacts by the result sender. diff --git a/result_server/routes/api.py b/result_server/routes/api.py index a7e1fdd..9106846 100644 --- a/result_server/routes/api.py +++ b/result_server/routes/api.py @@ -16,14 +16,16 @@ from utils.auth import verify_ingest_key, verify_trusted_proxy_auth from utils.audit_logging import audit_event from utils.environment_snapshots import index_environment_snapshot +from utils.measurement_artifacts import ( + is_profile_archive_basename, + normalize_measurement_artifact_basename, + stored_measurement_artifact_filename, +) from utils.rate_limit import rate_limited from utils.result_metadata_index import index_result_metadata api_bp = Blueprint("api", __name__) _TIMESTAMP_RE = re.compile(r"^\d{8}_\d{6}$") -_MEASUREMENT_ARTIFACT_BASENAME_RE = re.compile( - r"[A-Za-z0-9][A-Za-z0-9_.-]{0,127}\.(?:tgz|tar\.gz|json)" -) DEFAULT_MAX_ARCHIVE_MEMBER_SIZE = 1024 * 1024 * 1024 DEFAULT_MAX_ARCHIVE_TOTAL_EXTRACTED_SIZE = 1024 * 1024 * 1024 DEFAULT_MAX_ARCHIVE_MEMBER_COUNT = 4096 @@ -214,41 +216,6 @@ def _safe_basename(name): return name -def _normalize_measurement_artifact_basename( - value, - *, - error_message="Invalid measurement artifact path", -): - """Return a filename-safe results/ artifact basename, or None.""" - if value is None: - return None - - artifact_path = str(value).strip() - if artifact_path == "": - return None - if ( - os.path.isabs(artifact_path) - or "\\" in artifact_path - or artifact_path.startswith("../") - or "/../" in artifact_path - or artifact_path.endswith("/..") - ): - abort(400, description=error_message) - if not artifact_path.startswith("results/"): - abort(400, description=error_message) - - basename = os.path.basename(artifact_path) - if not _MEASUREMENT_ARTIFACT_BASENAME_RE.fullmatch(basename): - abort(400, description=error_message) - return basename - - -def _is_profile_archive_basename(basename): - return isinstance(basename, str) and ( - basename.endswith(".tgz") or basename.endswith(".tar.gz") - ) - - def _copy_uploaded_file(uploaded_file, save_path): """Write an uploaded file atomically.""" tmp_path = save_path + ".tmp" @@ -259,21 +226,6 @@ def _copy_uploaded_file(uploaded_file, save_path): os.rename(tmp_path, save_path) -def _measurement_artifact_filename(timestamp, uuid_str, artifact_basename): - if artifact_basename is None: - return _safe_basename(f"padata_{timestamp}_{uuid_str}.tgz") - if _is_profile_archive_basename(artifact_basename): - artifact_slug = ( - artifact_basename[:-7] - if artifact_basename.endswith(".tar.gz") - else artifact_basename[:-4] - ) - return _safe_basename(f"padata_{timestamp}_{uuid_str}_{artifact_slug}.tgz") - return _safe_basename( - f"measurement_artifact_{timestamp}_{uuid_str}_{artifact_basename}" - ) - - def _load_json_by_uuid(directory, field_path, uuid_value): """Return the first JSON payload whose target field matches the UUID.""" json_files = sorted( @@ -493,16 +445,21 @@ def ingest_measurement_artifact(): "RECEIVED_MEASUREMENT_ARTIFACTS_DIR", current_app.config.get("RECEIVED_PADATA_DIR", current_app.config["RECEIVED_DIR"]), ) - artifact_basename = _normalize_measurement_artifact_basename( - request.form.get("artifact_path") - ) - if artifact_basename is None and not _is_profile_archive_basename( + try: + artifact_basename = normalize_measurement_artifact_basename( + request.form.get("artifact_path") + ) + except ValueError: + abort(400, description="Invalid measurement artifact path") + if artifact_basename is None and not is_profile_archive_basename( uploaded_file.filename or "" ): abort(400, description="Missing measurement artifact path") if artifact_basename: - filename = _measurement_artifact_filename(timestamp, uuid_str, artifact_basename) + filename = _safe_basename( + stored_measurement_artifact_filename(timestamp, uuid_str, artifact_basename) + ) matched_files = ( [filename] if os.path.exists(os.path.join(received_dir, filename)) else [] ) @@ -520,7 +477,9 @@ def ingest_measurement_artifact(): save_path = old_file_path else: if not artifact_basename: - filename = _measurement_artifact_filename(timestamp, uuid_str, None) + filename = _safe_basename( + stored_measurement_artifact_filename(timestamp, uuid_str, None) + ) save_path = os.path.join(received_dir, filename) _copy_uploaded_file(uploaded_file, save_path) diff --git a/result_server/routes/results_detail_routes.py b/result_server/routes/results_detail_routes.py index 805f737..84dd90b 100644 --- a/result_server/routes/results_detail_routes.py +++ b/result_server/routes/results_detail_routes.py @@ -1,6 +1,5 @@ import json import os -import re from flask import abort, current_app, render_template, request, url_for from werkzeug.exceptions import Forbidden, NotFound @@ -31,6 +30,10 @@ serve_permitted_result_file, serve_public_padata_file, ) +from utils.measurement_artifacts import ( + is_measurement_artifact_filename, + stored_measurement_artifact_filename_from_path, +) from utils.result_records import ( format_numeric_value, format_result_timestamp, @@ -39,20 +42,6 @@ from utils.trigger_display import load_trigger_run_lookup, summarize_execution_trigger -PADATA_ARTIFACT_BASENAME_RE = re.compile( - r"[A-Za-z0-9][A-Za-z0-9_.-]{0,127}\.(?:tgz|tar\.gz)" -) -MEASUREMENT_ARTIFACT_BASENAME_RE = re.compile( - r"[A-Za-z0-9][A-Za-z0-9_.-]{0,127}\.(?:tgz|tar\.gz|json)" -) -MEASUREMENT_ARTIFACT_FILENAME_RE = re.compile( - r"^measurement_artifact_\d{8}_\d{6}_" - r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}_" - r"[A-Za-z0-9][A-Za-z0-9_.-]{0,127}\.(?:tgz|tar\.gz|json)$", - re.IGNORECASE, -) - - def register_results_detail_routes(results_bp): def public_surface(): return current_app.config.get("PUBLIC_PORTAL_MODE", False) @@ -104,7 +93,7 @@ def result_detail(filename): if is_public_surface else [ name for name in os.listdir(artifact_dir) - if _is_measurement_artifact_filename(name) + if is_measurement_artifact_filename(name) ] ) detail_context = build_result_detail_context( @@ -309,7 +298,7 @@ def show_result(filename): ) abort(404) - if _is_measurement_artifact_filename(filename): + if is_measurement_artifact_filename(filename): return serve_permitted_result_file( filename, current_app.config["RECEIVED_DIR"], @@ -353,10 +342,13 @@ def _list_result_padata_filenames(result, padata_dir): filenames = [] seen = set() for artifact_path in _iter_result_padata_artifact_paths(result): - artifact_slug = _padata_artifact_slug(artifact_path) - if not artifact_slug: + filename = stored_measurement_artifact_filename_from_path( + timestamp, + result_uuid, + artifact_path, + ) + if not filename or not filename.endswith(".tgz"): continue - filename = f"padata_{timestamp}_{result_uuid}_{artifact_slug}.tgz" if filename in seen: continue seen.add(filename) @@ -381,7 +373,11 @@ def _list_result_measurement_artifact_filenames(result, artifact_dir, *, include return filenames for artifact_path in _iter_result_timing_artifact_paths(result): - filename = _measurement_artifact_filename(timestamp, result_uuid, artifact_path) + filename = stored_measurement_artifact_filename_from_path( + timestamp, + result_uuid, + artifact_path, + ) if not filename or filename in seen: continue seen.add(filename) @@ -421,36 +417,5 @@ def _iter_result_timing_artifact_paths(result): yield path -def _padata_artifact_slug(artifact_path): - if not isinstance(artifact_path, str) or not artifact_path.startswith("results/"): - return "" - basename = os.path.basename(artifact_path) - if not PADATA_ARTIFACT_BASENAME_RE.fullmatch(basename): - return "" - return basename[:-7] if basename.endswith(".tar.gz") else basename[:-4] - - -def _measurement_artifact_filename(timestamp, result_uuid, artifact_path): - basename = _measurement_artifact_basename(artifact_path) - if not basename: - return "" - return f"measurement_artifact_{timestamp}_{result_uuid}_{basename}" - - -def _measurement_artifact_basename(artifact_path): - if not isinstance(artifact_path, str) or not artifact_path.startswith("results/"): - return "" - basename = os.path.basename(artifact_path) - if not MEASUREMENT_ARTIFACT_BASENAME_RE.fullmatch(basename): - return "" - return basename - - -def _is_measurement_artifact_filename(filename): - return filename.endswith(".tgz") or bool( - MEASUREMENT_ARTIFACT_FILENAME_RE.fullmatch(filename) - ) - - def _clean_result_value(value): return str(value or "").strip() diff --git a/result_server/tests/test_api_routes.py b/result_server/tests/test_api_routes.py index b7e4c12..1263911 100644 --- a/result_server/tests/test_api_routes.py +++ b/result_server/tests/test_api_routes.py @@ -503,6 +503,20 @@ def test_rejects_invalid_measurement_artifact_path(self, client, artifact_path): ) assert resp.status_code == 400 + def test_requires_artifact_path_for_non_archive_upload(self, client): + data = { + "id": "12345678-1234-1234-1234-123456789abc", + "timestamp": "20250101_120000", + "file": (io.BytesIO(b'{"timers": []}'), "qws_timing_CASE0.json"), + } + resp = client.post( + "/api/ingest/measurement-artifact", + data=data, + headers={"X-API-Key": API_KEY}, + content_type="multipart/form-data", + ) + assert resp.status_code == 400 + def test_missing_api_key_returns_401(self, client): resp = client.post( "/api/ingest/measurement-artifact", diff --git a/result_server/tests/test_measurement_artifacts.py b/result_server/tests/test_measurement_artifacts.py new file mode 100644 index 0000000..f7a26a8 --- /dev/null +++ b/result_server/tests/test_measurement_artifacts.py @@ -0,0 +1,107 @@ +import os +import sys + +import pytest + +sys.path.insert(0, os.path.join(os.path.dirname(__file__), "..")) + +from utils.measurement_artifacts import ( + PUBLIC_PADATA_FILENAME_RE, + is_measurement_artifact_filename, + measurement_artifact_basename_from_path, + normalize_measurement_artifact_basename, + profile_archive_filename_candidates, + stored_measurement_artifact_filename, + stored_measurement_artifact_filename_from_path, +) + + +UUID = "12345678-1234-1234-1234-123456789abc" +TIMESTAMP = "20250101_120000" + + +def test_normalizes_safe_results_artifact_paths(): + assert normalize_measurement_artifact_basename("results/qws_timing_CASE0.json") == ( + "qws_timing_CASE0.json" + ) + assert normalize_measurement_artifact_basename("results/padata_pairlist.tgz") == ( + "padata_pairlist.tgz" + ) + assert normalize_measurement_artifact_basename("") is None + assert normalize_measurement_artifact_basename(None) is None + + +@pytest.mark.parametrize( + "artifact_path", + [ + "../qws_timing.json", + "results/../qws_timing.json", + "/tmp/qws_timing.json", + "results/bad name.json", + "artifacts/qws_timing.json", + "results/qws_timing.txt", + ], +) +def test_rejects_unsafe_or_unsupported_artifact_paths(artifact_path): + with pytest.raises(ValueError): + normalize_measurement_artifact_basename(artifact_path) + assert measurement_artifact_basename_from_path(artifact_path) == "" + + +def test_builds_canonical_stored_names_for_legacy_profiles_and_timing_json(): + assert stored_measurement_artifact_filename(TIMESTAMP, UUID, None) == ( + f"padata_{TIMESTAMP}_{UUID}.tgz" + ) + assert stored_measurement_artifact_filename_from_path( + TIMESTAMP, + UUID, + "results/padata_pairlist.tgz", + ) == f"padata_{TIMESTAMP}_{UUID}_padata_pairlist.tgz" + assert stored_measurement_artifact_filename_from_path( + TIMESTAMP, + UUID, + "results/qws_timing_CASE0.json", + ) == f"measurement_artifact_{TIMESTAMP}_{UUID}_qws_timing_CASE0.json" + + +def test_profile_archive_candidates_keep_legacy_and_generic_names(): + assert profile_archive_filename_candidates( + TIMESTAMP, + UUID, + "results/padata_pairlist.tar.gz", + ) == [ + f"padata_{TIMESTAMP}_{UUID}_padata_pairlist.tgz", + f"measurement_artifact_{TIMESTAMP}_{UUID}_padata_pairlist.tar.gz", + ] + + +@pytest.mark.parametrize( + "filename", + [ + f"padata_{TIMESTAMP}_{UUID}.tgz", + f"padata_{TIMESTAMP}_{UUID}_padata_pairlist.tgz", + f"measurement_artifact_{TIMESTAMP}_{UUID}_qws_timing_CASE0.json", + ], +) +def test_recognizes_served_measurement_artifact_filenames(filename): + assert is_measurement_artifact_filename(filename) + + +def test_public_padata_filename_pattern_matches_full_uuid(): + assert PUBLIC_PADATA_FILENAME_RE.fullmatch(f"padata_{TIMESTAMP}_{UUID}.tgz") + assert PUBLIC_PADATA_FILENAME_RE.fullmatch( + f"padata_{TIMESTAMP}_{UUID}_padata_pairlist.tgz" + ) + + +@pytest.mark.parametrize( + "filename", + [ + "measurement_artifact_20250101_120000_qws_timing_CASE0.json", + f"measurement_artifact_{TIMESTAMP}_{UUID}_bad name.json", + "nested/debug_bundle.tgz", + "debug_bundle.json", + ], +) +def test_rejects_unserved_measurement_artifact_filenames(filename): + assert not is_measurement_artifact_filename(filename) diff --git a/result_server/utils/evidence_packet.py b/result_server/utils/evidence_packet.py index 20f5177..43c0bcb 100644 --- a/result_server/utils/evidence_packet.py +++ b/result_server/utils/evidence_packet.py @@ -7,6 +7,7 @@ from typing import Any from urllib.parse import urlsplit +from utils.measurement_artifacts import profile_archive_filename_candidates from utils.result_detail_view import ( build_cache_digest_help, build_cache_host_environment_help, @@ -283,10 +284,17 @@ def _profile_artifact_rows( uploaded = set(padata_filenames) rows = [] for section_name, artifact_path in _iter_profile_artifacts(result): - artifact_slug = _padata_artifact_slug(artifact_path) - if not artifact_slug: + archive_candidates = profile_archive_filename_candidates( + timestamp, + result_uuid, + artifact_path, + ) + if not archive_candidates: continue - archive = f"padata_{timestamp}_{result_uuid}_{artifact_slug}.tgz" + archive = next( + (candidate for candidate in archive_candidates if candidate in uploaded), + archive_candidates[0], + ) status = "available" if archive in uploaded else "not uploaded" archive_value = archive if archive in uploaded and padata_url_by_filename.get(archive): @@ -311,15 +319,6 @@ def _iter_profile_artifacts(result: dict[str, Any]): yield item_name, path -def _padata_artifact_slug(artifact_path: str) -> str: - if not isinstance(artifact_path, str) or not artifact_path.startswith("results/"): - return "" - basename = os.path.basename(artifact_path) - if not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9_.-]{0,127}\.(?:tgz|tar\.gz)", basename): - return "" - return basename[:-7] if basename.endswith(".tar.gz") else basename[:-4] - - def _build_cache_rows(build_cache: Any) -> list[tuple[str, Any]]: if not isinstance(build_cache, dict) or not build_cache: return [] diff --git a/result_server/utils/measurement_artifacts.py b/result_server/utils/measurement_artifacts.py new file mode 100644 index 0000000..70f9fe5 --- /dev/null +++ b/result_server/utils/measurement_artifacts.py @@ -0,0 +1,102 @@ +from __future__ import annotations + +import os +import re + + +MEASUREMENT_ARTIFACT_BASENAME_RE = re.compile( + r"[A-Za-z0-9][A-Za-z0-9_.-]{0,127}\.(?:tgz|tar\.gz|json)" +) +MEASUREMENT_ARTIFACT_FILENAME_RE = re.compile( + r"^measurement_artifact_\d{8}_\d{6}_" + r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}_" + r"[A-Za-z0-9][A-Za-z0-9_.-]{0,127}\.(?:tgz|tar\.gz|json)$", + re.IGNORECASE, +) +PUBLIC_PADATA_FILENAME_RE = re.compile( + r"^padata_\d{8}_\d{6}_" + r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}" + r"(?:_[A-Za-z0-9][A-Za-z0-9_.-]{0,127})?\.tgz$", + re.IGNORECASE, +) + + +def normalize_measurement_artifact_basename(value): + """Return the safe basename for a results/ artifact path.""" + if value is None: + return None + + artifact_path = str(value).strip() + if artifact_path == "": + return None + if ( + os.path.isabs(artifact_path) + or "\\" in artifact_path + or artifact_path.startswith("../") + or "/../" in artifact_path + or artifact_path.endswith("/..") + ): + raise ValueError("invalid measurement artifact path") + if not artifact_path.startswith("results/"): + raise ValueError("invalid measurement artifact path") + + basename = os.path.basename(artifact_path) + if not MEASUREMENT_ARTIFACT_BASENAME_RE.fullmatch(basename): + raise ValueError("invalid measurement artifact basename") + return basename + + +def measurement_artifact_basename_from_path(artifact_path): + try: + return normalize_measurement_artifact_basename(artifact_path) or "" + except ValueError: + return "" + + +def is_profile_archive_basename(basename): + return isinstance(basename, str) and ( + basename.endswith(".tgz") or basename.endswith(".tar.gz") + ) + + +def profile_archive_slug_from_basename(basename): + if not is_profile_archive_basename(basename): + return "" + return basename[:-7] if basename.endswith(".tar.gz") else basename[:-4] + + +def stored_measurement_artifact_filename(timestamp, result_uuid, artifact_basename=None): + if artifact_basename is None: + return f"padata_{timestamp}_{result_uuid}.tgz" + if is_profile_archive_basename(artifact_basename): + artifact_slug = profile_archive_slug_from_basename(artifact_basename) + return f"padata_{timestamp}_{result_uuid}_{artifact_slug}.tgz" + return f"measurement_artifact_{timestamp}_{result_uuid}_{artifact_basename}" + + +def stored_measurement_artifact_filename_from_path(timestamp, result_uuid, artifact_path): + artifact_basename = measurement_artifact_basename_from_path(artifact_path) + if not artifact_basename: + return "" + return stored_measurement_artifact_filename(timestamp, result_uuid, artifact_basename) + + +def profile_archive_filename_candidates(timestamp, result_uuid, artifact_path): + basename = measurement_artifact_basename_from_path(artifact_path) + if not is_profile_archive_basename(basename): + return [] + + return [ + stored_measurement_artifact_filename(timestamp, result_uuid, basename), + f"measurement_artifact_{timestamp}_{result_uuid}_{basename}", + ] + + +def is_measurement_artifact_filename(filename): + if not isinstance(filename, str) or os.path.basename(filename) != filename: + return False + if "/" in filename or "\\" in filename: + return False + return filename.endswith(".tgz") or bool( + MEASUREMENT_ARTIFACT_FILENAME_RE.fullmatch(filename) + ) diff --git a/result_server/utils/public_reuse.py b/result_server/utils/public_reuse.py index 30c2eea..1ee528c 100644 --- a/result_server/utils/public_reuse.py +++ b/result_server/utils/public_reuse.py @@ -9,6 +9,7 @@ from typing import Any from urllib.parse import urlsplit +from utils.measurement_artifacts import profile_archive_filename_candidates from utils.result_records import ( format_numeric_value, format_result_timestamp, @@ -504,14 +505,21 @@ def _profile_summary( uploaded = set(padata_filenames) artifacts = [] for section_name, artifact_path in _iter_profile_artifacts(result): - artifact_slug = _padata_artifact_slug(artifact_path) - if not artifact_slug: - continue result_uuid = _clean(result.get("_server_uuid")) timestamp = _clean(result.get("_server_timestamp")) if not result_uuid or not timestamp: continue - archive = f"padata_{timestamp}_{result_uuid}_{artifact_slug}.tgz" + archive_candidates = profile_archive_filename_candidates( + timestamp, + result_uuid, + artifact_path, + ) + if not archive_candidates: + continue + archive = next( + (candidate for candidate in archive_candidates if candidate in uploaded), + archive_candidates[0], + ) if archive not in uploaded: continue artifacts.append( @@ -718,15 +726,6 @@ def _iter_profile_artifacts(result: dict[str, Any]): yield item_name, path -def _padata_artifact_slug(artifact_path: str) -> str: - if not isinstance(artifact_path, str) or not artifact_path.startswith("results/"): - return "" - basename = os.path.basename(artifact_path) - if not re.fullmatch(r"[A-Za-z0-9][A-Za-z0-9_.-]{0,127}\.(?:tgz|tar\.gz)", basename): - return "" - return basename[:-7] if basename.endswith(".tar.gz") else basename[:-4] - - def _safe_relative_path(value: Any) -> str: text = _clean(value) if not text or os.path.isabs(text) or "\\" in text: diff --git a/result_server/utils/result_detail_view.py b/result_server/utils/result_detail_view.py index ddb85f4..90ae6c3 100644 --- a/result_server/utils/result_detail_view.py +++ b/result_server/utils/result_detail_view.py @@ -1,8 +1,9 @@ -import os -import re - from flask import url_for +from utils.measurement_artifacts import ( + profile_archive_filename_candidates, + stored_measurement_artifact_filename_from_path, +) from utils.result_records import build_labeled_value_rows, format_numeric_value from utils.trigger_display import summarize_execution_trigger @@ -80,11 +81,6 @@ }, } -MEASUREMENT_ARTIFACT_BASENAME_RE = re.compile( - r"[A-Za-z0-9][A-Za-z0-9_.-]{0,127}\.(?:tgz|tar\.gz|json)" -) - - def build_cache_host_environment_help(context="matched"): return HOST_ENVIRONMENT_FINGERPRINT_HELP.get( context, HOST_ENVIRONMENT_FINGERPRINT_HELP["matched"] @@ -252,7 +248,11 @@ def _build_profile_measurement_artifact_rows(result, timestamp, result_uuid, upl if not isinstance(artifact, dict) or artifact.get("type") != "file_reference": continue artifact_path = artifact.get("path") or "" - candidates = _profile_artifact_filenames(timestamp, result_uuid, artifact_path) + candidates = profile_archive_filename_candidates( + timestamp, + result_uuid, + artifact_path, + ) if not candidates: continue filename = _choose_uploaded_filename(candidates, uploaded) @@ -288,10 +288,13 @@ def _build_timing_measurement_artifact_rows(result, timestamp, result_uuid, uplo if artifact.get("type") != "file_reference": continue artifact_path = str(artifact.get("path") or "").strip() - basename = _measurement_artifact_basename(artifact_path) - if not basename: + filename = stored_measurement_artifact_filename_from_path( + timestamp, + result_uuid, + artifact_path, + ) + if not filename: continue - filename = f"measurement_artifact_{timestamp}_{result_uuid}_{basename}" label = str(observation.get("id") or f"Observation {index}") rows.append({ "kind": "Timing observation", @@ -314,29 +317,6 @@ def _choose_uploaded_filename(candidates, uploaded): return candidates[0] -def _profile_artifact_filenames(timestamp, result_uuid, artifact_path): - basename = _measurement_artifact_basename(artifact_path) - if not basename or not (basename.endswith(".tgz") or basename.endswith(".tar.gz")): - return [] - - artifact_slug = basename[:-7] if basename.endswith(".tar.gz") else basename[:-4] - return [ - f"padata_{timestamp}_{result_uuid}_{artifact_slug}.tgz", - f"measurement_artifact_{timestamp}_{result_uuid}_{basename}", - ] - - -def _measurement_artifact_basename(artifact_path): - if not isinstance(artifact_path, str): - return "" - if not artifact_path.startswith("results/"): - return "" - basename = os.path.basename(artifact_path) - if not MEASUREMENT_ARTIFACT_BASENAME_RE.fullmatch(basename): - return "" - return basename - - def _build_quality_rows(quality): if not quality: return [] diff --git a/result_server/utils/result_file.py b/result_server/utils/result_file.py index 0f08708..607e87a 100644 --- a/result_server/utils/result_file.py +++ b/result_server/utils/result_file.py @@ -7,6 +7,10 @@ from flask import Response, abort, send_from_directory, session +from utils.measurement_artifacts import ( + MEASUREMENT_ARTIFACT_FILENAME_RE, + PUBLIC_PADATA_FILENAME_RE, +) from utils.session_user_context import get_session_user_context @@ -14,18 +18,6 @@ r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}", re.IGNORECASE, ) -PUBLIC_PADATA_FILENAME_RE = re.compile( - r"^padata_\d{8}_\d{6}_" - r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}" - r"(?:_[A-Za-z0-9][A-Za-z0-9_.-]{0,127})?\.tgz$", - re.IGNORECASE, -) -MEASUREMENT_ARTIFACT_FILENAME_RE = re.compile( - r"^measurement_artifact_\d{8}_\d{6}_" - r"[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}_" - r"[A-Za-z0-9][A-Za-z0-9_.-]{0,127}\.(?:tgz|tar\.gz|json)$", - re.IGNORECASE, -) def load_result_file(filename: str, save_dir: str):