refactor: move json validation into JSON

* json parameter validation is done completely within the JSON
  ParamType.

* further an validation about existing keys was added.

* a deep validation with key -> value is still missing, therefore the
  pid_identifier could not be validated yet.

repository_cli/click_options.py

@@ -13,6 +13,8 @@
 
 from click import BOOL, STRING, Choice, File, option
 
+from .click_param_types import JSON
+
 T = TypeVar("T")
 
 
@@ -82,6 +84,7 @@ def option_identifier(
         "--identifier",
         "-i",
         required=required,
+        type=JSON(validate=["identifier", "scheme"]),
         help="metadata identifier as JSON",
     )
 
@@ -99,6 +102,7 @@ def option_pid_identifier(
     return option(
         "--pid-identifier",
         required=required,
+        type=JSON(),
         help="pid identifier as JSON",
     )
 

repository_cli/click_param_types.py

@@ -9,7 +9,7 @@
 
 
 import sys
-from json import JSONDecodeError, load
+from json import JSONDecodeError, load, loads
 from pathlib import Path
 from typing import Any, Optional
 
@@ -18,28 +18,46 @@
 from .types import Color
 
 
+def _error_msg(art: str, key: str) -> str:
+    """Construct error message."""
+    error_msgs = {
+        "validate": f"The given json does not validate, key: '{key}' does not exists",
+    }
+    return error_msgs[art]
+
+
 class JSON(ParamType):
     """JSON provides the ability to load a json from a string or a file."""
 
     name = "JSON"
 
+    def __init__(self, validate: list[str] = None) -> None:
+        """Construct Json ParamType."""
+        self.validate = validate
+
     def convert(
         self,
         value: Any,  # noqa: ANN401
         param: Optional["Parameter"],  # noqa: ARG002
         ctx: Optional["Context"],  # noqa: ARG002
     ) -> Any:  # noqa: ANN401
         """The method converts the json-file to the dictionary representation."""
-        if not Path(value).is_file():
-            secho("ERROR - please look up if the file path is correct.", fg=Color.error)
-            sys.exit()
-
         try:
-            with Path(value).open("r", encoding="utf8") as file_pointer:
-                obj = load(file_pointer)
+            if Path(value).is_file():
+                with Path(value).open("r", encoding="utf8") as file_pointer:
+                    obj = load(file_pointer)
+            else:
+                obj = loads(value)
         except JSONDecodeError as error:
-            secho("ERROR - Invalid JSON provided.", fg=Color.error)
+            msg = "ERROR - Invalid JSON provided. Check file path or json string."
+            secho(msg, fg=Color.error)
             secho(f"  error: {error.args[0]}", fg=Color.error)
             sys.exit()
-        else:
-            return obj
+
+        if self.validate:
+            for key in self.validate:
+                if key not in obj:
+                    secho(_error_msg("validate", key), fg=Color.error)
+                    sys.exit()
+
+        return obj

repository_cli/records.py

@@ -303,16 +303,9 @@ def replace_pid(pid: str, pid_identifier: str) -> None:
 
     example call:
         invenio repository records pids replace -p "fcze8-4vx33"
-        --pid-identifier ' { "doi": {
-        "identifier": "10.48436/fcze8-4vx33", "provider": "unmanaged" }}'
+        --pid-identifier '{"doi": {
+        "identifier": "10.48436/fcze8-4vx33", "provider": "unmanaged"}}'
     """
-    try:
-        pid_identifier_json = json.loads(pid_identifier)
-    except Exception as error:
-        secho(str(error), fg=Color.error)
-        secho("pid_identifier is not valid JSON", fg=Color.error)
-        return
-
     service = get_records_service()
     identity = get_identity(permission_name="system_process", role_name="admin")
 
@@ -323,13 +316,13 @@ def replace_pid(pid: str, pid_identifier: str) -> None:
     old_data = service.read(id_=pid, identity=identity).data.copy()
     new_data = old_data.copy()
     pids = new_data.get("pids", {})
-    pid_key = list(pid_identifier_json.keys())[0]
+    pid_key = list(pid_identifier.keys())[0]
 
     if pids.get(pid_key, None) is None:
         secho(f"'{pid}' does not have pid identifier '{pid_key}'", fg=Color.warning)
         return
 
-    pids[pid_key] = pid_identifier_json.get(pid_key)
+    pids[pid_key] = pid_identifier.get(pid_key)
     new_data["pids"] = pids
 
     try:
@@ -383,13 +376,6 @@ def add_identifier(identifier: str, pid: str) -> None:
         invenio repository records identifiers add -p "fcze8-4vx33"
         -i '{ "identifier": "10.48436/fcze8-4vx33", "scheme": "doi"}'
     """
-    try:
-        identifier_json = json.loads(identifier)
-    except Exception as error:
-        secho(str(error), fg=Color.error)
-        secho("identifier is not valid JSON", fg=Color.error)
-        return
-
     service = get_records_service()
     identity = get_identity("system_process", role_name="admin")
 
@@ -400,14 +386,14 @@ def add_identifier(identifier: str, pid: str) -> None:
     record_data = service.read(id_=pid, identity=identity).data.copy()
 
     current_identifiers = record_data["metadata"].get("identifiers", [])
-    current_schemes = [_["scheme"] for _ in current_identifiers]
-    scheme = identifier_json["scheme"]
+    current_schemes = [ci["scheme"] for ci in current_identifiers]
+    scheme = identifier["scheme"]
     if scheme in current_schemes:
         secho(f"scheme '{scheme}' already in identifiers", fg=Color.error)
         return
 
     old_data = record_data.copy()
-    current_identifiers.append(identifier_json)
+    current_identifiers.append(identifier)
     record_data["metadata"]["identifiers"] = current_identifiers
 
     try:
@@ -431,13 +417,6 @@ def replace_identifier(identifier: str, pid: str) -> None:
         invenio repository records identifiers replace -p "fcze8-4vx33"
         -i '{ "identifier": "10.48436/fcze8-4vx33", "scheme": "doi"}'
     """
-    try:
-        identifier_json = json.loads(identifier)
-    except Exception as error:
-        secho(str(error), fg=Color.error)
-        secho("identifier is not valid JSON", fg=Color.error)
-        return
-
     service = get_records_service()
     identity = get_identity("system_process", role_name="admin")
 
@@ -447,11 +426,11 @@ def replace_identifier(identifier: str, pid: str) -> None:
 
     record_data = service.read(id_=pid, identity=identity).data.copy()
     current_identifiers = record_data["metadata"].get("identifiers", [])
-    scheme = identifier_json["scheme"]
+    scheme = identifier["scheme"]
     replaced = False
     for index, current_identifier in enumerate(current_identifiers):
         if current_identifier["scheme"] == scheme:
-            current_identifiers[index] = identifier_json
+            current_identifiers[index] = identifier
             replaced = True
             break
 

tests/test_rdmrecords_identifiers.py

@@ -87,8 +87,13 @@ def test_add_identifier_wrong_identifier_type(
         add_identifier,
         ["--pid", r_id, "--identifier", "this is not a dict"],
     )
+
+    expected_error_msg = (
+        "ERROR - Invalid JSON provided. Check file path or json string."
+    )
+
     assert response.exit_code == 0
-    assert "identifier is not valid JSON" in response.output
+    assert expected_error_msg in response.output
 
 
 def test_add_identifiers_record_not_found(
@@ -135,6 +140,24 @@ def test_replace_identifier_scheme_does_not_exist(
     assert f"scheme '{identifier['scheme']}' not in identifiers" in response.output
 
 
+def test_replace_identifier_schema_missing(
+    app_initialized: Flask,
+    create_record: RecordItem,
+) -> None:
+    """Test replace identifier scheme missing."""
+    runner = app_initialized.test_cli_runner()
+    r_id = create_record.id
+    response = runner.invoke(
+        replace_identifier,
+        ["--pid", r_id, "--identifier", '{"identifier": "10.48436/fcze8-4vx33"}'],
+    )
+    expected_error_msg = (
+        "The given json does not validate, key: 'scheme' does not exists"
+    )
+    assert response.exit_code == 0
+    assert expected_error_msg in response.output
+
+
 def test_replace_identifier_wrong_identifier_type(
     app_initialized: Flask,
     create_record: RecordItem,
@@ -146,8 +169,12 @@ def test_replace_identifier_wrong_identifier_type(
         replace_identifier,
         ["--pid", r_id, "--identifier", "this is not a dict"],
     )
+    expected_error_msg = (
+        "ERROR - Invalid JSON provided. Check file path or json string."
+    )
+
     assert response.exit_code == 0
-    assert "identifier is not valid JSON" in response.output
+    assert expected_error_msg in response.output
 
 
 def test_replace_identifiers_record_not_found(

tests/test_rdmrecords_pids.py

@@ -69,8 +69,12 @@ def test_replace_pid_wrong_identifier_type(
         replace_pid,
         ["--pid", r_id, "--pid-identifier", "this is not a dict"],
     )
+
+    expected_error_msg = (
+        "ERROR - Invalid JSON provided. Check file path or json string."
+    )
     assert response.exit_code == 0
-    assert "pid_identifier is not valid JSON" in response.output
+    assert expected_error_msg in response.output
 
 
 def test_replace_pid_record_not_found(