From 63327e7d884574df0c3c86b95e85002e84e5b1bc Mon Sep 17 00:00:00 2001
From: Qixiang Wan <qwan@redhat.com>
Date: Fri, 14 Apr 2017 13:12:35 +0800
Subject: [PATCH] checks: Fix anyOf validator yield ValidationError on
 ConfigOptionWarning

We have some hooks yield ConfigOptionWarning. When it happens within
anyOf validator, anyOf validator yield ValidationError and reports the
config as incorrect. We need to overwrite it to pass not break.

Fixes: #598
Merges: #599
Signed-off-by: Qixiang Wan <qwan@redhat.com>
---
 pungi/checks.py      | 25 +++++++++++++++++++++-
 tests/test_checks.py | 51 +++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/pungi/checks.py b/pungi/checks.py
index 437b3b04..db0b677d 100644
--- a/pungi/checks.py
+++ b/pungi/checks.py
@@ -349,12 +349,35 @@ def _extend_with_default_and_alias(validator_class):
             for error in validate_type(validator, properties, instance, schema):
                 yield error
 
+    def _validate_any_of(validator, anyOf, instance, schema):
+        """
+        Overwrite jsonschema's anyOf validator to not yield ValidationError when
+        ConfigOptionWarning is found.
+        """
+        all_errors = []
+
+        for index, subschema in enumerate(anyOf):
+            errs = list(validator.descend(instance, subschema, schema_path=index))
+            warnings = [err for err in errs if isinstance(err, ConfigOptionWarning)]
+            errors = [err for err in errs if err not in warnings]
+            if not errors:
+                for warning in warnings:
+                    yield warning
+                break
+            all_errors.extend(errors)
+        else:
+            yield jsonschema.ValidationError(
+                "%r is not valid under any of the given schemas" % (instance,),
+                context=all_errors,
+            )
+
     return jsonschema.validators.extend(
         validator_class, {"properties": _set_defaults,
                           "deprecated": error_on_deprecated,
                           "type": validate_regex_type,
                           "required": _validate_required,
-                          "additionalProperties": _validate_additional_properties},
+                          "additionalProperties": _validate_additional_properties,
+                          "anyOf": _validate_any_of},
     )
 
 
diff --git a/tests/test_checks.py b/tests/test_checks.py
index cd4b009d..84866141 100644
--- a/tests/test_checks.py
+++ b/tests/test_checks.py
@@ -16,7 +16,6 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), '..'))
 
 from pungi import checks
 
-
 class CheckDependenciesTestCase(unittest.TestCase):
 
     def dont_find(self, paths):
@@ -481,6 +480,56 @@ class TestSchemaValidator(unittest.TestCase):
         self.assertEqual(config.get("release_name", None), "dummy product")
         self.assertEqual(config.get("repo", None), ["http://url/to/repo", "Server", "Client"])
 
+    @mock.patch('pungi.checks._make_schema')
+    def test_anyof_validator_not_raise_our_warnings_as_error(self, make_schema):
+        # https://pagure.io/pungi/issue/598
+        schema = {
+            "$schema": "http://json-schema.org/draft-04/schema#",
+            "title": "Pungi Configuration",
+            "type": "object",
+            "definitions": {
+                "live_image_config": {
+                    "type": "object",
+                    "properties": {
+                        "repo": {
+                            "type": "string",
+                            "append": "repo_from",
+                        },
+                    },
+                },
+            },
+            "properties": {
+                "live_images": checks._variant_arch_mapping({
+                    "anyOf": [
+                        {"$ref": "#/definitions/live_image_config"},
+                        {
+                            "type": "array",
+                            "items": {
+                                "$ref": "#/definitions/live_image_config"
+                            }
+                        }
+                    ]
+                }),
+            },
+        }
+        make_schema.return_value = schema
+
+        string = """
+        live_images = [
+            ('^Spins$', {
+                'armhfp': {
+                    'repo_from': 'Everything',
+            }}),
+        ]
+        """
+        config = self._load_conf_from_string(string)
+        errors, warnings = checks.validate(config)
+        self.assertEqual(len(errors), 0)
+        self.assertEqual(len(warnings), 2)
+        self.assertRegexpMatches(warnings[0], r"^WARNING: Config option 'repo_from' is deprecated, its value will be appended to option 'repo'.*")
+        self.assertRegexpMatches(warnings[1], r"^WARNING: Config option 'repo' is not found, but 'repo_from' is specified, value from 'repo_from' is now added as 'repo'.*")
+        self.assertEqual(config.get("live_images")[0][1]['armhfp']['repo'], 'Everything')
+
 
 class TestUmask(unittest.TestCase):
     def setUp(self):