Use JJBConfig in YamlParser.

This commit sees JJBConfig start to take the form it ought to have,
namely using attributes to represent different logical sections of
configuration that target specific subsystems of JJB.

It also moves ConfigParser data retrieval from
jenkins_jobs.modules.helpers.get_value_from_yaml_or_config_file() to
jenkins_jobs.config.JJBConfig.get_module_config()

TODO: Add JJBConfig tests to validate the behavior of JJBConfig in
specific circumstances.

Change-Id: I053d165559f5325a2f40b239117a86e6d0f3ef37

jenkins_jobs/builder.py

@@ -225,15 +225,16 @@ class Jenkins(object):
 
 
 class Builder(object):
-    def __init__(self, jenkins_url, jenkins_user, jenkins_password,
+    def __init__(self, jjb_config):
-                 config=None, jenkins_timeout=_DEFAULT_TIMEOUT,
+        self.jenkins = Jenkins(jjb_config.jenkins['url'],
-                 ignore_cache=False, flush_cache=False, plugins_list=None):
+                               jjb_config.jenkins['user'],
-        self.jenkins = Jenkins(jenkins_url, jenkins_user, jenkins_password,
+                               jjb_config.jenkins['password'],
-                               jenkins_timeout)
+                               jjb_config.jenkins['timeout'])
-        self.cache = CacheStorage(jenkins_url, flush=flush_cache)
+        self.cache = CacheStorage(jjb_config.jenkins['url'],
-        self.global_config = config
+                                  flush=jjb_config.builder['flush_cache'])
-        self.ignore_cache = ignore_cache
+        self._plugins_list = jjb_config.builder['plugins_info']
-        self._plugins_list = plugins_list
+
+        self.jjb_config = jjb_config
 
     @property
     def plugins_list(self):
@@ -242,7 +243,7 @@ class Builder(object):
         return self._plugins_list
 
     def load_files(self, fn):
-        self.parser = YamlParser(self.global_config, self.plugins_list)
+        self.parser = YamlParser(self.jjb_config, self.plugins_list)
 
         # handle deprecated behavior, and check that it's not a file like
         # object as these may implement the '__iter__' attribute.
@@ -337,7 +338,9 @@ class Builder(object):
     @parallelize
     def changed(self, job):
         md5 = job.md5()
-        changed = self.ignore_cache or self.cache.has_changed(job.name, md5)
+
+        changed = (self.jjb_config.builder['ignore_cache'] or
+                   self.cache.has_changed(job.name, md5))
         if not changed:
             logger.debug("'{0}' has not changed".format(job.name))
         return changed

jenkins_jobs/cli/entry.py

@@ -73,12 +73,20 @@ class JenkinsJobs(object):
         self._parse_additional()
         self.jjb_config.validate()
 
+    def _set_config(self, target, option):
+        """
+        Sets the option in target only if the given option was explicitly set
+        """
+        opt_val = getattr(self.options, option, None)
+        if opt_val is not None:
+            target[option] = opt_val
+
     def _parse_additional(self):
-        for opt in ['ignore_cache', 'user', 'password',
+
-                    'allow_empty_variables']:
+        self._set_config(self.jjb_config.builder, 'ignore_cache')
-            opt_val = getattr(self.options, opt, None)
+        self._set_config(self.jjb_config.yamlparser, 'allow_empty_variables')
-            if opt_val is not None:
+        self._set_config(self.jjb_config.jenkins, 'user')
-                setattr(self.jjb_config, opt, opt_val)
+        self._set_config(self.jjb_config.jenkins, 'password')
 
         if getattr(self.options, 'plugins_info_path', None) is not None:
             with io.open(self.options.plugins_info_path, 'r',
@@ -87,7 +95,7 @@ class JenkinsJobs(object):
             if not isinstance(plugins_info, list):
                 self.parser.error("{0} must contain a Yaml list!".format(
                                   self.options.plugins_info_path))
-            self.jjb_config.plugins_info = plugins_info
+            self.jjb_config.builder['plugins_info'] = plugins_info
 
         if getattr(self.options, 'path', None):
             if hasattr(self.options.path, 'read'):
@@ -118,17 +126,8 @@ class JenkinsJobs(object):
                 self.options.path = paths
 
     def execute(self):
-        config = self.jjb_config.config_parser
         options = self.options
-
+        builder = Builder(self.jjb_config)
-        builder = Builder(config.get('jenkins', 'url'),
-                          self.jjb_config.user,
-                          self.jjb_config.password,
-                          self.jjb_config.config_parser,
-                          jenkins_timeout=self.jjb_config.timeout,
-                          ignore_cache=self.jjb_config.ignore_cache,
-                          flush_cache=options.flush_cache,
-                          plugins_list=self.jjb_config.plugins_info)
 
         if options.command == 'delete':
             for job in options.name:

jenkins_jobs/config.py

@@ -15,6 +15,7 @@
 
 # Manage JJB Configuration sources, defaults, and access.
 
+from collections import defaultdict
 import io
 import logging
 import os
@@ -118,12 +119,18 @@ class JJBConfig(object):
         self.config_parser = config_parser
 
         self.ignore_cache = False
+        self.flush_cache = False
         self.user = None
         self.password = None
         self.plugins_info = None
         self.timeout = builder._DEFAULT_TIMEOUT
         self.allow_empty_variables = None
 
+        self.jenkins = defaultdict(None)
+        self.builder = defaultdict(None)
+        self.yamlparser = defaultdict(None)
+        self.hipchat = defaultdict(None)
+
         self._setup()
 
     def _init_defaults(self):
@@ -205,27 +212,88 @@ class JJBConfig(object):
         self.recursive = config.getboolean('job_builder', 'recursive')
         self.excludes = config.get('job_builder', 'exclude').split(os.pathsep)
 
+        # The way we want to do things moving forward:
+        self.jenkins['url'] = config.get('jenkins', 'url')
+        self.jenkins['user'] = self.user
+        self.jenkins['password'] = self.password
+        self.jenkins['timeout'] = self.timeout
+
+        self.builder['ignore_cache'] = self.ignore_cache
+        self.builder['flush_cache'] = self.flush_cache
+        self.builder['plugins_info'] = self.plugins_info
+
+        # keep descriptions ? (used by yamlparser)
+        keep_desc = False
+        if (config and config.has_section('job_builder') and
+                config.has_option('job_builder', 'keep_descriptions')):
+            keep_desc = config.getboolean('job_builder',
+                                          'keep_descriptions')
+        self.yamlparser['keep_descriptions'] = keep_desc
+
+        # figure out the include path (used by yamlparser)
+        path = ["."]
+        if (config and config.has_section('job_builder') and
+                config.has_option('job_builder', 'include_path')):
+            path = config.get('job_builder',
+                              'include_path').split(':')
+        self.yamlparser['include_path'] = path
+
+        # allow duplicates?
+        allow_duplicates = False
+        if config and config.has_option('job_builder', 'allow_duplicates'):
+            allow_duplicates = config.getboolean('job_builder',
+                                                 'allow_duplicates')
+        self.yamlparser['allow_duplicates'] = allow_duplicates
+
+        # allow empty variables?
+        self.yamlparser['allow_empty_variables'] = (
+            self.allow_empty_variables or
+            config and config.has_section('job_builder') and
+            config.has_option('job_builder', 'allow_empty_variables') and
+            config.getboolean('job_builder', 'allow_empty_variables'))
+
     def validate(self):
         config = self.config_parser
 
         # Inform the user as to what is likely to happen, as they may specify
         # a real jenkins instance in test mode to get the plugin info to check
         # the XML generated.
-        if self.user is None and self.password is None:
+        if self.jenkins['user'] is None and self.jenkins['password'] is None:
             logger.info("Will use anonymous access to Jenkins if needed.")
-        elif (self.user is not None and self.password is None) or (
+        elif ((self.jenkins['user'] is not None and
-                self.user is None and self.password is not None):
+               self.jenkins['password'] is None) or
+              (self.jenkins['user'] is None and
+               self.jenkins['password'] is not None)):
             raise JenkinsJobsException(
                 "Cannot authenticate to Jenkins with only one of User and "
                 "Password provided, please check your configuration."
             )
 
-        if (self.plugins_info is not None and
+        if (self.builder['plugins_info'] is not None and
-                not isinstance(self.plugins_info, list)):
+                not isinstance(self.builder['plugins_info'], list)):
             raise JenkinsJobsException("plugins_info must contain a list!")
 
         # Temporary until yamlparser is refactored to query config object
-        if self.allow_empty_variables is not None:
+        if self.yamlparser['allow_empty_variables'] is not None:
             config.set('job_builder',
                        'allow_empty_variables',
-                       str(self.allow_empty_variables))
+                       str(self.yamlparser['allow_empty_variables']))
+
+    def get_module_config(self, section, key):
+        """ Given a section name and a key value, return the value assigned to
+        the key in the JJB .ini file if it exists, otherwise emit a warning
+        indicating that the value is not set. Default value returned if no
+        value is set in the file will be a blank string.
+        """
+        result = ''
+        try:
+            result = self.config_parser.get(
+                section, key
+            )
+        except (configparser.NoSectionError, configparser.NoOptionError,
+                JenkinsJobsException) as e:
+            logger.warning("You didn't set a " + key +
+                           " neither in the yaml job definition nor in" +
+                           " the " + section + " section, blank default" +
+                           " value will be applied:\n{0}".format(e))
+        return result

jenkins_jobs/modules/helpers.py

@@ -12,11 +12,8 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
-import logging
 import xml.etree.ElementTree as XML
 
-from six.moves import configparser
-
 from jenkins_jobs.errors import InvalidAttributeError
 from jenkins_jobs.errors import JenkinsJobsException
 from jenkins_jobs.errors import MissingAttributeError
@@ -254,19 +251,10 @@ def findbugs_settings(xml_parent, data):
 
 
 def get_value_from_yaml_or_config_file(key, section, data, parser):
-    logger = logging.getLogger(__name__)
     result = data.get(key, '')
+    jjb_config = parser.jjb_config
     if result == '':
-        try:
+        result = jjb_config.get_module_config(section, key)
-            result = parser.config.get(
-                section, key
-            )
-        except (configparser.NoSectionError, configparser.NoOptionError,
-                JenkinsJobsException) as e:
-            logger.warning("You didn't set a " + key +
-                           " neither in the yaml job definition nor in" +
-                           " the " + section + " section, blank default" +
-                           " value will be applied:\n{0}".format(e))
     return result
 
 

jenkins_jobs/modules/publishers.py

@@ -409,8 +409,8 @@ def trigger_parameterized_builds(parser, xml_parent, data):
     ]
 
     try:
-        if parser.config.getboolean('__future__',
+        if parser.jjb_config.config_parser.getboolean('__future__',
-                                    'param_order_from_yaml'):
+                                                      'param_order_from_yaml'):
             orig_order = None
     except six.moves.configparser.NoSectionError:
         pass

jenkins_jobs/parser.py

@@ -70,27 +70,17 @@ def combination_matches(combination, match_combinations):
 
 
 class YamlParser(object):
-    def __init__(self, config=None, plugins_info=None):
+    def __init__(self, jjb_config=None, plugins_info=None):
         self.data = {}
         self.jobs = []
         self.xml_jobs = []
-        self.config = config
-        self.registry = ModuleRegistry(self.config, plugins_info)
-        self.path = ["."]
-        if self.config:
-            if config.has_section('job_builder') and \
-                    config.has_option('job_builder', 'include_path'):
-                self.path = config.get('job_builder',
-                                       'include_path').split(':')
-        self.keep_desc = self.get_keep_desc()
 
-    def get_keep_desc(self):
+        self.jjb_config = jjb_config
-        keep_desc = False
+        self.keep_desc = jjb_config.yamlparser['keep_descriptions']
-        if self.config and self.config.has_section('job_builder') and \
+        self.path = jjb_config.yamlparser['include_path']
-                self.config.has_option('job_builder', 'keep_descriptions'):
+
-            keep_desc = self.config.getboolean('job_builder',
+        self.registry = ModuleRegistry(jjb_config.config_parser,
-                                               'keep_descriptions')
+                                       plugins_info)
-        return keep_desc
 
     def parse_fp(self, fp):
         # wrap provided file streams to ensure correct encoding used
@@ -129,8 +119,7 @@ class YamlParser(object):
 
     def _handle_dups(self, message):
 
-        if not (self.config and self.config.has_section('job_builder') and
+        if not self.jjb_config.yamlparser['allow_duplicates']:
-                self.config.getboolean('job_builder', 'allow_duplicates')):
             logger.error(message)
             raise JenkinsJobsException(message)
         else:
@@ -311,19 +300,14 @@ class YamlParser(object):
                 logger.debug('Excluding combination %s', str(params))
                 continue
 
-            allow_empty_variables = self.config \
-                and self.config.has_section('job_builder') \
-                and self.config.has_option(
-                    'job_builder', 'allow_empty_variables') \
-                and self.config.getboolean(
-                    'job_builder', 'allow_empty_variables')
-
             for key in template.keys():
                 if key not in params:
                     params[key] = template[key]
 
             params['template-name'] = template_name
-            expanded = deep_format(template, params, allow_empty_variables)
+            expanded = deep_format(
+                template, params,
+                self.jjb_config.yamlparser['allow_empty_variables'])
 
             job_name = expanded.get('name')
             if jobs_glob and not matches(job_name, jobs_glob):

tests/base.py

@@ -131,14 +131,15 @@ class BaseTestCase(LoggingFixture):
 
     def _get_config(self):
         jjb_config = JJBConfig(self.conf_filename)
+        jjb_config.validate()
 
-        return jjb_config.config_parser
+        return jjb_config
 
     def test_yaml_snippet(self):
         if not self.in_filename:
             return
 
-        config = self._get_config()
+        jjb_config = self._get_config()
 
         expected_xml = self._read_utf8_content()
         yaml_content = self._read_yaml_content(self.in_filename)
@@ -151,7 +152,7 @@ class BaseTestCase(LoggingFixture):
             self.addDetail("plugins-info",
                            text_content(str(plugins_info)))
 
-        parser = YamlParser(config, plugins_info)
+        parser = YamlParser(jjb_config, plugins_info)
 
         pub = self.klass(parser.registry)
 

tests/builder/test_builder.py

@@ -14,9 +14,11 @@
 # License for the specific language governing permissions and limitations
 # under the License.
 
+from jenkins_jobs.config import JJBConfig
 import jenkins_jobs.builder
 from tests.base import LoggingFixture
 from tests.base import mock
+
 from testtools import TestCase
 
 
@@ -24,11 +26,10 @@ from testtools import TestCase
 class TestCaseTestBuilder(LoggingFixture, TestCase):
     def setUp(self):
         super(TestCaseTestBuilder, self).setUp()
-        self.builder = jenkins_jobs.builder.Builder(
+        jjb_config = JJBConfig()
-            'http://jenkins.example.com',
+        jjb_config.builder['plugins_info'] = ['plugin1', 'plugin2']
-            'doesnot', 'matter',
+        jjb_config.validate()
-            plugins_list=['plugin1', 'plugin2'],
+        self.builder = jenkins_jobs.builder.Builder(jjb_config)
-        )
 
     def test_plugins_list(self):
         self.assertEqual(self.builder.plugins_list, ['plugin1', 'plugin2'])

tests/cmd/subcommands/test_update.py

@@ -18,6 +18,7 @@ import os
 import six
 
 from jenkins_jobs import builder
+from jenkins_jobs.config import JJBConfig
 from tests.base import mock
 from tests.cmd.test_cmd import CmdTestsBase
 
@@ -79,9 +80,13 @@ class UpdateTests(CmdTestsBase):
         jobs = ['old_job001', 'old_job002', 'unmanaged']
         extra_jobs = [{'name': name} for name in jobs]
 
-        builder_obj = builder.Builder('http://jenkins.example.com',
+        jjb_config = JJBConfig()
-                                      'doesnot', 'matter',
+        jjb_config.jenkins['url'] = 'http://example.com'
-                                      plugins_list={})
+        jjb_config.jenkins['user'] = 'doesnot'
+        jjb_config.jenkins['password'] = 'matter'
+        jjb_config.builder['plugins_info'] = []
+        jjb_config.validate()
+        builder_obj = builder.Builder(jjb_config)
 
         # get the instance created by mock and redirect some of the method
         # mocks to call real methods on a the above test object.

tests/cmd/test_config.py

@@ -83,11 +83,12 @@ class TestConfigs(CmdTestsBase):
                                    'settings_from_config.ini')
         args = ['--conf', config_file, 'test', 'dummy.yaml']
         jenkins_jobs = entry.JenkinsJobs(args)
-        self.assertEqual(jenkins_jobs.jjb_config.ignore_cache, True)
+        jjb_config = jenkins_jobs.jjb_config
-        self.assertEqual(jenkins_jobs.jjb_config.user, "jenkins_user")
+        self.assertEqual(jjb_config.jenkins['user'], "jenkins_user")
-        self.assertEqual(jenkins_jobs.jjb_config.password, "jenkins_password")
+        self.assertEqual(jjb_config.jenkins['password'], "jenkins_password")
-        self.assertEqual(jenkins_jobs.jjb_config.config_parser.get(
+        self.assertEqual(jjb_config.builder['ignore_cache'], True)
-            'job_builder', 'allow_empty_variables'), "True")
+        self.assertEqual(
+            jjb_config.yamlparser['allow_empty_variables'], True)
 
     def test_config_options_overriden_by_cli(self):
         """
@@ -98,8 +99,9 @@ class TestConfigs(CmdTestsBase):
                 '--ignore-cache', '--allow-empty-variables',
                 'test', 'dummy.yaml']
         jenkins_jobs = entry.JenkinsJobs(args)
-        self.assertEqual(jenkins_jobs.jjb_config.ignore_cache, True)
+        jjb_config = jenkins_jobs.jjb_config
-        self.assertEqual(jenkins_jobs.jjb_config.user, "myuser")
+        self.assertEqual(jjb_config.jenkins['user'], "myuser")
-        self.assertEqual(jenkins_jobs.jjb_config.password, "mypassword")
+        self.assertEqual(jjb_config.jenkins['password'], "mypassword")
-        self.assertEqual(jenkins_jobs.jjb_config.config_parser.get(
+        self.assertEqual(jjb_config.builder['ignore_cache'], True)
-            'job_builder', 'allow_empty_variables'), "True")
+        self.assertEqual(
+            jjb_config.yamlparser['allow_empty_variables'], True)

tests/localyaml/test_localyaml.py

@@ -22,6 +22,7 @@ from testtools import TestCase
 from yaml.composer import ComposerError
 
 from jenkins_jobs import builder
+from jenkins_jobs.config import JJBConfig
 from tests.base import get_scenarios
 from tests.base import JsonTestCase
 from tests.base import LoggingFixture
@@ -76,6 +77,11 @@ class TestCaseLocalYamlIncludeAnchors(LoggingFixture, TestCase):
         files = ["custom_same_anchor-001-part1.yaml",
                  "custom_same_anchor-001-part2.yaml"]
 
-        b = builder.Builder("http://example.com", "jenkins", None,
+        jjb_config = JJBConfig()
-                            plugins_list=[])
+        jjb_config.jenkins['url'] = 'http://example.com'
+        jjb_config.jenkins['user'] = 'jenkins'
+        jjb_config.jenkins['password'] = 'password'
+        jjb_config.builder['plugins_info'] = []
+        jjb_config.validate()
+        b = builder.Builder(jjb_config)
         b.load_files([os.path.join(self.fixtures_path, f) for f in files])