Improve metadata checking errors

biosimdb_interface/form/utils.py

@@ -11,6 +11,15 @@
 from biosimdb_interface.schema.webform import get_simulation_metadata
 
 
+class FormConversionError(ValueError):
+    """Raised when form values can't be parsed; holds every message in ``errors``."""
+
+    def __init__(self, errors, heading="Form conversion failed:"):
+        self.errors = list(errors)
+        self.heading = heading
+        super().__init__(heading + "\n" + "\n".join(self.errors))
+
+
 def make_upload_tmpdir(prefix):
     upload_root = current_app.config["UPLOAD_FOLDER"]
     os.makedirs(upload_root, exist_ok=True)
@@ -100,6 +109,25 @@ def _get_typehint_map():
     return result
 
 
+def _parse_vector(raw, field):
+    """Parse '1, 2, 3' to a flat list, or '1,2; 3,4' to a list of rows."""
+    rows = [r for r in re.split(r"[;\n]", raw) if r.strip()]
+    parsed = []
+    for i, row in enumerate(rows, start=1):
+        try:
+            parsed.append([float(x) for x in row.split(",") if x.strip()])
+        except ValueError:
+            raise ValueError(
+                f"{field}: row {i} {row.strip()!r} is not a comma-separated list of numbers"
+            ) from None
+    if len(rows) <= 1:
+        return parsed[0] if parsed else []
+    lengths = {len(r) for r in parsed}
+    if len(lengths) > 1:
+        raise ValueError(f"{field}: rows have different lengths {sorted(lengths)}")
+    return parsed
+
+
 def form_to_json(form):
     """Convert flat HTML form data into a nested dictionary.
 
@@ -115,6 +143,7 @@ def form_to_json(form):
     """
     typehint_map = _get_typehint_map()
     data = {}
+    errors = []
     # Use lists() so repeated keys (e.g. multiselect name="x[]") are preserved.
     if hasattr(form, "lists"):
         key_values_iter = form.lists()
@@ -136,7 +165,11 @@ def form_to_json(form):
         if parts[-1] == "vector_value":
             raw = values[0] if values else ""
             if isinstance(raw, str) and raw:
-                value = [float(x.strip()) for x in raw.split(",") if x.strip()]
+                try:
+                    value = _parse_vector(raw, ".".join(parts))
+                except ValueError as exc:
+                    errors.append(str(exc))
+                    continue
             else:
                 value = raw
             _set_nested(data, parts, value)
@@ -153,12 +186,16 @@ def form_to_json(form):
                     try:
                         value = int(value)
                     except (ValueError, TypeError):
-                        pass
+                        errors.append(
+                            f"{'.'.join(parts)}: {value!r} is not a valid {typehint}"
+                        )
                 elif typehint == "float":
                     try:
                         value = float(value)
                     except (ValueError, TypeError):
-                        pass
+                        errors.append(
+                            f"{'.'.join(parts)}: {value!r} is not a valid {typehint}"
+                        )
                 elif typehint == "boolean":
                     value = bool(value)
             converted_values.append(value)
@@ -171,6 +208,8 @@ def form_to_json(form):
                 value_out = converted_values[0] if converted_values else ""
 
         _set_nested(data, parts, value_out)
+    if errors:
+        raise FormConversionError(errors)
     return data
 
 

biosimdb_interface/form/webform.py

@@ -3,8 +3,14 @@
 import os
 
 import requests
-from biosim_extractor.metadata.convertpopulated import convert_populated_metadata_units
-from biosim_extractor.metadata.validatemetadata import validate_metadata
+from biosim_extractor.metadata.convertpopulated import (
+    UnitConversionError,
+    convert_populated_metadata_units,
+)
+from biosim_extractor.metadata.validatemetadata import (
+    MetadataValidationError,
+    validate_metadata,
+)
 from flask import (
     current_app,
     flash,
@@ -15,6 +21,7 @@
     session,
     url_for,
 )
+from werkzeug.exceptions import HTTPException
 
 from biosimdb_interface.login.community_invite import invite_user
 from biosimdb_interface.schema.webform import WEBFORM_SCHEMA, get_simulation_metadata
@@ -30,7 +37,7 @@
     save_pending_submission,
     verify_cached_file_meta,
 )
-from .utils import form_to_json, remove_empty_fields
+from .utils import FormConversionError, form_to_json, remove_empty_fields
 from .validation import validate_with_mdanalysis
 from .workflows import WorkflowNotFound
 
@@ -41,118 +48,108 @@ def _get_workflow():
     return current_app.extensions["workflow_store"].get(workflow_id)
 
 
-@form_bp.route("/webform", methods=["GET", "POST"])
-def webform():
-    """Render the metadata form and handle save/submit actions.
+def _handle_save(json_form, tmpdir):
+    """Return the validated metadata as a JSON download response."""
+    return jsonify({"success": True, "data": json_form})
 
-    On POST, validates uploaded files, converts submitted metadata to standard
-    units, removes empty fields, and validates the result against the BioSim
-    schema. ``save`` returns the validated JSON to the browser. ``submit`` saves
-    uploaded files plus the validated JSON for deferred Invenio upload, then
-    starts login if needed.
-    """
-    clear_client_state = False
-    token = session.get("access_token")
-
-    # an abandoned/failed login leaves a pending submit; discard it on return
-    if request.method == "GET" and session.pop("force_clear_client_state", False):
-        clear_client_state = True
-
-    if request.method == "POST":
-        try:
-            workflow = _get_workflow()
-        except WorkflowNotFound as exc:
-            return jsonify({"validation_errors": [str(exc)]}), 400
-
-        tmpdir = workflow.tmpdir
-        topo_path, traj_files = load_extracted_files(tmpdir)
-
-        action = (
-            "save"
-            if "save" in request.form
-            else "submit"
-            if "submit" in request.form
-            else None
+
+def _handle_submit(json_form, tmpdir):
+    """Save metadata and start or defer the submission process."""
+    save_pending_submission(json_form, tmpdir)
+    workflow_id = request.form["workflow_id"]
+    if not session.get("access_token"):
+        session["post_login_redirect"] = url_for(
+            "form.resume_submit", workflow_id=workflow_id
         )
+        return redirect(url_for("login.login"))
+    return render_template("form/loading.html", workflow_id=workflow_id)
+
+
+_HANDLERS = {"save": _handle_save, "submit": _handle_submit}
 
-        if action == "submit":
-            if not tmpdir or not topo_path or not traj_files:
-                return jsonify(
-                    {
-                        "validation_errors": [
-                            "Please extract metadata before submitting."
-                        ]
-                    }
-                ), 400
-
-            ok, err = verify_cached_file_meta(tmpdir)
-            if not ok:
-                return jsonify({"validation_errors": [err]}), 400
-
-        if action in ["save", "submit"]:
-            if not tmpdir or not topo_path or not traj_files:
-                return jsonify(
-                    {
-                        "validation_errors": [
-                            "Please extract metadata before submitting."
-                        ]
-                    }
-                ), 400
-
-        # check files can be read with mda
-        mda_error = validate_with_mdanalysis(topo_path, traj_files)
-
-        if mda_error:
-            return jsonify({"validation_errors": [mda_error]})
-        if action in ["save", "submit"]:
-            # include file info in output, ro-crate?
-            form_values = request.form.copy()
-            # remove workflow_id from form
-            form_values.pop("workflow_id", None)
-            json_form = form_to_json(form_values)
-            json_form = remove_empty_fields(json_form)
-
-            # convert to standard units
-            json_form = convert_populated_metadata_units(json_form)
-
-            if action == "save":
-                json_form["files"] = extract_uploaded_file_metadata(tmpdir)
-            biosimschema_path = os.getenv("BIOSIM_SCHEMA_PATH", "")
-
-            validation_errors = []
-            try:
-                validate_metadata(json_form, biosimschema_path, strict=True)
-            except ValueError as e:
-                validation_errors = str(e).splitlines()
-
-            if validation_errors:
-                return jsonify(
-                    {
-                        "validation_errors": validation_errors,
-                    }
-                )
-
-            if action == "submit":
-                save_pending_submission(json_form, tmpdir)
-                if not token:
-                    session["post_login_redirect"] = url_for(
-                        "form.resume_submit",
-                        workflow_id=request.form["workflow_id"],
-                    )
-                    return redirect(url_for("login.login"))
-
-                return render_template(
-                    "form/loading.html",
-                    workflow_id=request.form["workflow_id"],
-                )
-
-            if action == "save":
-                return jsonify({"success": True, "data": json_form})
-
-    schema = {**WEBFORM_SCHEMA}
-    schema["data"] = {
-        **WEBFORM_SCHEMA["data"],
-        "simulation_metadata": get_simulation_metadata(),
+
+@form_bp.errorhandler(Exception)
+def handle_unexpected_error(exc):
+    """Log an unexpected blueprint error and return a JSON error response."""
+    if isinstance(exc, HTTPException):
+        return exc
+
+    current_app.logger.exception("Unhandled error in form blueprint")
+    message = (
+        f"{type(exc).__name__}: {exc}"
+        if current_app.debug
+        else "An unexpected server error occurred. Please try again."
+    )
+    return jsonify({"validation_errors": [message]}), 500
+
+
+def _errors_response(errors, status=200):
+    """JSON response in the shape the front end expects."""
+    return jsonify({"validation_errors": errors}), status
+
+
+def _group(exc):
+    """Convert a validation exception into a heading-and-errors group."""
+    return {"heading": exc.heading, "errors": exc.errors}
+
+
+def _build_metadata(form, tmpdir, action):
+    """Form -> validated metadata dict. Returns (json_form, error_groups).
+
+    Three checks of form data:
+    1. use the webform fields and expected types to check fields are correctly formatted.
+    2. Check fields that are to be converted to standard units are correctly formatted.
+    3. Check fields conform to linkml schema.
+
+    """
+    form_values = form.copy()
+    form_values.pop("workflow_id", None)
+
+    try:
+        # check fields match typehint formats
+        json_form = remove_empty_fields(form_to_json(form_values))
+    except FormConversionError as e:
+        return None, [_group(e)]  # nothing usable to convert/validate
+
+    groups = []
+    try:
+        # convert to standard units via extractor
+        json_form = convert_populated_metadata_units(json_form)
+    except UnitConversionError as e:
+        groups.append(_group(e))
+
+    if action == "save":
+        json_form["files"] = extract_uploaded_file_metadata(tmpdir)
+
+    try:
+        # validate metadata against linkml schema
+        validate_metadata(json_form, os.getenv("BIOSIM_SCHEMA_PATH", ""), strict=True)
+    except MetadataValidationError as e:
+        groups.append(_group(e))
+    return json_form, groups
+
+
+def _check_ready(action, tmpdir, topo_path, traj_files):
+    """Return an error message if the request can't proceed, else None."""
+    if action in ("save", "submit") and not (tmpdir and topo_path and traj_files):
+        return "Please extract metadata before submitting."
+    if action == "submit":
+        ok, err = verify_cached_file_meta(tmpdir)
+        if not ok:
+            return err
+    return None
+
+
+@form_bp.route("/webform", methods=["GET"])
+def webform():
+    """Render the metadata form."""
+    clear_client_state = bool(session.pop("force_clear_client_state", False))
+    schema = {
+        **WEBFORM_SCHEMA,
+        "data": {
+            **WEBFORM_SCHEMA["data"],
+            "simulation_metadata": get_simulation_metadata(),
+        },
     }
     return render_template(
         "form/webform.html",
@@ -163,6 +160,34 @@ def webform():
     )
 
 
+@form_bp.route("/webform", methods=["POST"])
+def webform_post():
+    """Validate and handle a metadata save or submission request."""
+    try:
+        workflow = _get_workflow()
+    except WorkflowNotFound as exc:
+        return _errors_response([str(exc)], 400)
+
+    tmpdir = workflow.tmpdir
+    topo_path, traj_files = load_extracted_files(tmpdir)
+    action = next((a for a in ("save", "submit") if a in request.form), None)
+
+    if msg := _check_ready(action, tmpdir, topo_path, traj_files):
+        return _errors_response([msg], 400)
+
+    if msg := validate_with_mdanalysis(topo_path, traj_files):
+        return _errors_response([msg])
+
+    if action not in ("save", "submit"):
+        return _errors_response(["Unknown action."], 400)
+
+    json_form, groups = _build_metadata(request.form, tmpdir, action)
+    if groups:
+        return _errors_response(groups)
+
+    return _HANDLERS[action](json_form, tmpdir)
+
+
 @form_bp.route("/resume_submit")
 def resume_submit():
     """Resume a pending submission after successful login.