pre-upload: Check json file in project_public repo
Add a check to make sure the battery config json file matches
the rule we require in BCIC.
BUG=b:356543760
TEST=(1)./run_tests pre-upload_unittest.py -k CheckJsonKeyPrefixTest
(2)manual test: upload an invalid json and fail
(3)manual test: upload a valid json and pass
(4)manual test: upload a prefix error json and fail
Change-Id: Ia2edd518301931370b8a27775bdf79f73ca43530
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/repohooks/+/5884233
Tested-by: Tzu-Min Sun <jimmysun@chromium.org>
Commit-Queue: Tzu-Min Sun <jimmysun@chromium.org>
Reviewed-by: Alex Klein <saklein@chromium.org>
diff --git a/pre-upload.py b/pre-upload.py
index a382a43..6b4bf61 100755
--- a/pre-upload.py
+++ b/pre-upload.py
@@ -18,6 +18,7 @@
import fnmatch
import functools
import itertools
+import json
import os
from pathlib import Path
import re
@@ -2836,6 +2837,62 @@
return None
+def _check_json_key_prefix(project: Project, commit: str):
+ """Checks that modified .json files are valid battery configs for BCIC.
+
+ This hook performs two main checks on modified .json files:
+ 1. Validates that the file content is parseable as valid JSON.
+ 2. Ensures that no pair of top-level keys in the JSON data are prefixes of
+ each other.
+
+ Args:
+ project: The Project to look at
+ commit: The commit to look at
+
+ Returns:
+ None if all checks pass, otherwise a HookFailure object with details
+ of the errors.
+ """
+ if project.name != "chromiumos/project":
+ return None
+
+ files = _filter_files(
+ _get_affected_files(commit, relative=True), [r".*\.json$"]
+ )
+
+ json_error_list = []
+ prefix_error_dict = {}
+ for file in files:
+ try:
+ json_file = json.loads(_get_file_content(file, commit))
+ keys = list(json_file.keys())
+ file_prefix_errors = []
+ for i, key1 in enumerate(keys):
+ for key2 in keys[i + 1 :]:
+ if key1.startswith(key2) or key2.startswith(key1):
+ file_prefix_errors.append(sorted([key1, key2]))
+ if file_prefix_errors:
+ prefix_error_dict[file] = file_prefix_errors
+
+ except json.JSONDecodeError as e:
+ json_error_list.append(f"{file}: {e}")
+
+ if json_error_list:
+ return errors.HookFailure("Invalid JSON file(s):", json_error_list)
+
+ if prefix_error_dict:
+ prefix_error_list = []
+ for file, prefix_errors in prefix_error_dict.items():
+ for key1, key2 in prefix_errors:
+ prefix_error_list.append(
+ f"{file}: Key '{key1}' is a prefix of Key '{key2}'"
+ )
+ return errors.HookFailure(
+ "JSON file(s) with prefix errors:", prefix_error_list
+ )
+ return None
+
+
def _check_cl_size(
_project: Project, commit: str, options: Sequence[str] = ()
) -> None:
@@ -2895,6 +2952,7 @@
_check_exec_files,
_check_for_uprev,
_check_git_cl_presubmit,
+ _check_json_key_prefix,
_check_keywords,
_check_layout_conf,
_check_no_extra_blank_lines,
diff --git a/pre-upload_unittest.py b/pre-upload_unittest.py
index 66e893f..7d0a467 100644
--- a/pre-upload_unittest.py
+++ b/pre-upload_unittest.py
@@ -3424,3 +3424,69 @@
"README.md:[Hook Overrides]"
),
)
+
+
+class CheckJsonKeyPrefixTest(PreUploadTestCase, cros_test_lib.TempDirTestCase):
+ """Tests for _check_json_key_prefix."""
+
+ def setUp(self):
+ self.project = pre_upload.Project(
+ name="chromiumos/project", dir=self.tempdir, remote=None
+ )
+ # Ugly hack since self.tempdir is not in the chroot.
+ self.PatchObject(path_util, "ToChrootPath", side_effect=lambda x: x)
+ self.file_mock = self.PatchObject(pre_upload, "_get_affected_files")
+ self.content_mock = self.PatchObject(pre_upload, "_get_file_content")
+
+ def testJsonFormat(self):
+ """Verify that we can check the format of a json file."""
+ self.file_mock.return_value = [
+ f"{self.project.dir}/path/to/my/json_files/foo.json"
+ ]
+ self.content_mock.return_value = """
+{
+ "a": 1,
+ "b": 2,
+ "c": 3
+}
+ """
+ ret = pre_upload._check_json_key_prefix(self.project, "COMMIT")
+ self.assertIsNone(ret)
+
+ def testInvalidJsonFormat(self):
+ """Verify that we can check the format of a json file."""
+ self.file_mock.return_value = [
+ f"{self.project.dir}/path/to/my/json_files/foo.json"
+ ]
+ self.content_mock.return_value = """
+{
+ "a": 1,
+ "b": 2,
+ "c": 3,
+}
+ """
+ ret = pre_upload._check_json_key_prefix(self.project, "COMMIT")
+ self.assertTrue(ret)
+ self.assertEqual("Invalid JSON file(s):", ret.msg)
+
+ def testPrefixError(self):
+ """Verify that no pair of keys in the json file are prefixes."""
+ self.file_mock.return_value = [
+ f"{self.project.dir}/path/to/my/json_files/foo.json"
+ ]
+ self.content_mock.return_value = """
+{
+ "abc": 1,
+ "ab": 2,
+ "c": 3
+}
+ """
+ ret = pre_upload._check_json_key_prefix(self.project, "COMMIT")
+ self.assertTrue(ret)
+ self.assertEqual(
+ ret.items,
+ [
+ f"{self.project.dir}/path/to/my/json_files/foo.json: "
+ "Key 'ab' is a prefix of Key 'abc'"
+ ],
+ )
