From 36bd52621f67a2ec560fa0e0e4ae892be28cc033 Mon Sep 17 00:00:00 2001 From: Anne Redulla Date: Tue, 19 Sep 2023 02:55:56 +0000 Subject: [PATCH] [ssci] Support alias for Shipped field Bug: b:297823626 Change-Id: Ib4be88567040d147f6cdba4f6c7d2b37a0f3898b Reviewed-on: https://chromium-review.googlesource.com/c/chromium/tools/depot_tools/+/4871939 Reviewed-by: Rachael Newitt Commit-Queue: Anne Redulla --- metadata/dependency_metadata.py | 42 ++++++++++-- metadata/fields/known.py | 2 + .../data/README.chromium.test.multi-valid | 12 ++++ metadata/tests/dependency_metadata_test.py | 68 +++++++++++++++++++ 4 files changed, 117 insertions(+), 7 deletions(-) diff --git a/metadata/dependency_metadata.py b/metadata/dependency_metadata.py index 0f5082e3b..79d746208 100644 --- a/metadata/dependency_metadata.py +++ b/metadata/dependency_metadata.py @@ -36,6 +36,15 @@ class DependencyMetadata: known_fields.SHIPPED, } + # Aliases for fields, where: + # * key is the alias field; and + # * value is the main field to which it should be mapped. + # Note: if both the alias and main fields are specified in metadata, + # the value from the alias field will be used. + _FIELD_ALIASES = { + known_fields.SHIPPED_IN_CHROMIUM: known_fields.SHIPPED, + } + def __init__(self): # The record of all entries added, including repeated fields. self._entries: List[Tuple[str, str]] = [] @@ -122,6 +131,32 @@ class DependencyMetadata: ]) results.append(error) + # Process alias fields. + sources = {} + for alias_field, main_field in self._FIELD_ALIASES.items(): + if alias_field in self._metadata: + # Validate the value that was present for the main field + # before overwriting it with the alias field value. + if main_field in self._metadata: + main_value = self._metadata.get(main_field) + field_result = main_field.validate(main_value) + if field_result: + field_result.set_tag(tag="field", + value=main_field.get_name()) + results.append(field_result) + + self._metadata[main_field] = self._metadata[alias_field] + sources[main_field] = alias_field + self._metadata.pop(alias_field) + + # Validate values for all present fields. + for field, value in self._metadata.items(): + source_field = sources.get(field) or field + field_result = source_field.validate(value) + if field_result: + field_result.set_tag(tag="field", value=source_field.get_name()) + results.append(field_result) + # Check required fields are present. required_fields = self._assess_required_fields() for field in required_fields: @@ -150,13 +185,6 @@ class DependencyMetadata: ) results.append(error) - # Validate values for all present fields. - for field, value in self._metadata.items(): - field_result = field.validate(value) - if field_result: - field_result.set_tag(tag="field", value=field.get_name()) - results.append(field_result) - # Check existence of the license file(s) on disk. license_file_value = self._metadata.get(known_fields.LICENSE_FILE) if license_file_value is not None: diff --git a/metadata/fields/known.py b/metadata/fields/known.py index 5b18bd8c0..aa065ff9c 100644 --- a/metadata/fields/known.py +++ b/metadata/fields/known.py @@ -33,6 +33,7 @@ LOCAL_MODIFICATIONS = field_types.FreeformTextField("Local Modifications", # Yes/no fields. SECURITY_CRITICAL = field_types.YesNoField("Security Critical") SHIPPED = field_types.YesNoField("Shipped") +SHIPPED_IN_CHROMIUM = field_types.YesNoField("Shipped in Chromium") LICENSE_ANDROID_COMPATIBLE = field_types.YesNoField( "License Android Compatible") @@ -55,6 +56,7 @@ ALL_FIELDS = ( LICENSE_FILE, SECURITY_CRITICAL, SHIPPED, + SHIPPED_IN_CHROMIUM, LICENSE_ANDROID_COMPATIBLE, CPE_PREFIX, DESCRIPTION, diff --git a/metadata/tests/data/README.chromium.test.multi-valid b/metadata/tests/data/README.chromium.test.multi-valid index 50e7dda4d..18db89113 100644 --- a/metadata/tests/data/README.chromium.test.multi-valid +++ b/metadata/tests/data/README.chromium.test.multi-valid @@ -30,3 +30,15 @@ License: Apache, 2.0 and MIT License File: LICENSE Security Critical: yes Shipped: yes + +-------------------- DEPENDENCY DIVIDER -------------------- + +Name: Test-C README for Chromium metadata +Short Name: metadata-test-valid-again +URL: https://www.example.com/metadata +Version: 1.0.12 +Date: 2020-12-03 +License: Apache, 2.0 and MIT +License File: LICENSE +Security Critical: yes +Shipped in Chromium: yes diff --git a/metadata/tests/dependency_metadata_test.py b/metadata/tests/dependency_metadata_test.py index aa71d2943..4c5383bc1 100644 --- a/metadata/tests/dependency_metadata_test.py +++ b/metadata/tests/dependency_metadata_test.py @@ -44,6 +44,74 @@ class DependencyValidationTest(unittest.TestCase): self.assertTrue(isinstance(results[0], vr.ValidationError)) self.assertEqual(results[0].get_reason(), "There is a repeated field.") + def test_only_alias_field(self): + """Check that an alias field can be used for a main field.""" + dependency = dm.DependencyMetadata() + dependency.add_entry(known_fields.URL.get_name(), + "https://www.example.com") + dependency.add_entry(known_fields.NAME.get_name(), + "Test alias field used") + dependency.add_entry(known_fields.VERSION.get_name(), "1.0.0") + dependency.add_entry(known_fields.LICENSE_FILE.get_name(), "LICENSE") + dependency.add_entry(known_fields.LICENSE.get_name(), "Public domain") + # Use Shipped in Chromium instead of Shipped. + dependency.add_entry(known_fields.SHIPPED_IN_CHROMIUM.get_name(), "no") + dependency.add_entry(known_fields.SECURITY_CRITICAL.get_name(), "no") + + results = dependency.validate( + source_file_dir=os.path.join(_THIS_DIR, "data"), + repo_root_dir=_THIS_DIR, + ) + self.assertEqual(len(results), 0) + + def test_alias_overwrite_invalid_field(self): + """Check that the value for an overwritten field (from an alias + field) is still validated. + """ + dependency = dm.DependencyMetadata() + dependency.add_entry(known_fields.URL.get_name(), + "https://www.example.com") + dependency.add_entry(known_fields.NAME.get_name(), + "Test alias field overwrite") + dependency.add_entry(known_fields.VERSION.get_name(), "1.0.0") + dependency.add_entry(known_fields.LICENSE_FILE.get_name(), "LICENSE") + dependency.add_entry(known_fields.LICENSE.get_name(), "Public domain") + dependency.add_entry(known_fields.SHIPPED_IN_CHROMIUM.get_name(), "no") + dependency.add_entry(known_fields.SHIPPED.get_name(), "test") + dependency.add_entry(known_fields.SECURITY_CRITICAL.get_name(), "no") + + results = dependency.validate( + source_file_dir=os.path.join(_THIS_DIR, "data"), + repo_root_dir=_THIS_DIR, + ) + self.assertEqual(len(results), 1) + self.assertEqual(results[0].get_reason(), "Shipped is invalid.") + + def test_alias_invalid_field_attributed(self): + """Check that an invalid value from an alias field is attributed + to that alias field. + """ + dependency = dm.DependencyMetadata() + dependency.add_entry(known_fields.URL.get_name(), + "https://www.example.com") + dependency.add_entry(known_fields.NAME.get_name(), + "Test alias field error attributed") + dependency.add_entry(known_fields.VERSION.get_name(), "1.0.0") + dependency.add_entry(known_fields.LICENSE_FILE.get_name(), "LICENSE") + dependency.add_entry(known_fields.LICENSE.get_name(), "Public domain") + dependency.add_entry(known_fields.SHIPPED_IN_CHROMIUM.get_name(), + "test") + dependency.add_entry(known_fields.SHIPPED.get_name(), "yes") + dependency.add_entry(known_fields.SECURITY_CRITICAL.get_name(), "no") + + results = dependency.validate( + source_file_dir=os.path.join(_THIS_DIR, "data"), + repo_root_dir=_THIS_DIR, + ) + self.assertEqual(len(results), 1) + self.assertEqual(results[0].get_reason(), + "Shipped in Chromium is invalid.") + def test_versioning_field(self): """Check that a validation error is returned for insufficient versioning info."""