node: Add 'get_str_list' on 'MappingNode'bschubert/api-improvements

`mapping.get_sequence(...).as_str_list()` is a very common
pattern seen both in plugins and the core. Adding a helper to reduce
the number of operations will make usage smoother

17 files changed, 55 insertions, 32 deletions

diff --git a/doc/bst2html.py b/doc/bst2html.py
index 71d497eda..b3204b8d8 100755
--- a/doc/bst2html.py
+++ b/doc/bst2html.py
@@ -368,7 +368,7 @@ def run_session(description, tempdir, source_cache, palette, config_file, force)
             # not a source distribution, no need to complain
             pass
 
-    remove_files = desc.get_sequence('remove-files', default=[]).as_str_list()
+    remove_files = desc.get_str_list('remove-files', default=[])
     for remove_file in remove_files:
         remove_file = os.path.join(desc_dir, remove_file)
         remove_file = os.path.realpath(remove_file)
diff --git a/src/buildstream/_loader/loader.py b/src/buildstream/_loader/loader.py
index 207de9adb..a17eaf9e1 100644
--- a/src/buildstream/_loader/loader.py
+++ b/src/buildstream/_loader/loader.py
@@ -487,7 +487,7 @@ class Loader():
                                    node.get_mapping(Symbol.CONFIG, default={}),
                                    node.get_mapping(Symbol.VARIABLES, default={}),
                                    node.get_mapping(Symbol.ENVIRONMENT, default={}),
-                                   node.get_sequence(Symbol.ENV_NOCACHE, default=[]).as_str_list(),
+                                   node.get_str_list(Symbol.ENV_NOCACHE, default=[]),
                                    node.get_mapping(Symbol.PUBLIC, default={}),
                                    node.get_mapping(Symbol.SANDBOX, default={}),
                                    element_kind == 'junction')
diff --git a/src/buildstream/_options/optionenum.py b/src/buildstream/_options/optionenum.py
index d1a7a85c9..3d5053639 100644
--- a/src/buildstream/_options/optionenum.py
+++ b/src/buildstream/_options/optionenum.py
@@ -45,7 +45,7 @@ class OptionEnum(Option):
 
         node.validate_keys(valid_symbols)
 
-        self.values = node.get_sequence('values', default=[]).as_str_list()
+        self.values = node.get_str_list('values', default=[])
         if not self.values:
             raise LoadError(LoadErrorReason.INVALID_DATA,
                             "{}: No values specified for {} option '{}'"
diff --git a/src/buildstream/_options/optionflags.py b/src/buildstream/_options/optionflags.py
index 80dd1b55d..64149d28e 100644
--- a/src/buildstream/_options/optionflags.py
+++ b/src/buildstream/_options/optionflags.py
@@ -93,4 +93,4 @@ class OptionFlags(Option):
     def load_valid_values(self, node):
         # Allow the more descriptive error to raise when no values
         # exist rather than bailing out here (by specifying default_value)
-        return node.get_sequence('values', default=[]).as_str_list()
+        return node.get_str_list('values', default=[])
diff --git a/src/buildstream/_plugincontext.py b/src/buildstream/_plugincontext.py
index 2442e306f..6b5d84e3b 100644
--- a/src/buildstream/_plugincontext.py
+++ b/src/buildstream/_plugincontext.py
@@ -137,7 +137,7 @@ class PluginContext():
             loaded_dependency = False
 
             for origin in self._plugin_origins:
-                if kind not in origin.get_sequence('plugins').as_str_list():
+                if kind not in origin.get_str_list('plugins'):
                     continue
 
                 if origin.get_str('origin') == 'local':
diff --git a/src/buildstream/_project.py b/src/buildstream/_project.py
index 6a2c0f347..95afc78b5 100644
--- a/src/buildstream/_project.py
+++ b/src/buildstream/_project.py
@@ -591,10 +591,10 @@ class Project():
 
         defaults = pre_config_node.get_mapping('defaults')
         defaults.validate_keys(['targets'])
-        self._default_targets = defaults.get_sequence("targets").as_str_list()
+        self._default_targets = defaults.get_str_list("targets")
 
         # Fatal warnings
-        self._fatal_warnings = pre_config_node.get_sequence('fatal-warnings', default=[]).as_str_list()
+        self._fatal_warnings = pre_config_node.get_str_list('fatal-warnings', default=[])
 
         self.loader = Loader(self._context, self,
                              parent=parent_loader, fetch_subprojects=fetch_subprojects)
@@ -668,7 +668,7 @@ class Project():
 
         # Load sandbox environment variables
         self.base_environment = config.get_mapping('environment')
-        self.base_env_nocache = config.get_sequence('environment-nocache').as_str_list()
+        self.base_env_nocache = config.get_str_list('environment-nocache')
 
         # Load sandbox configuration
         self._sandbox = config.get_mapping('sandbox')
@@ -700,7 +700,7 @@ class Project():
         # Parse shell options
         shell_options = config.get_mapping('shell')
         shell_options.validate_keys(['command', 'environment', 'host-files'])
-        self._shell_command = shell_options.get_sequence('command').as_str_list()
+        self._shell_command = shell_options.get_str_list('command')
 
         # Perform environment expansion right away
         shell_environment = shell_options.get_mapping('environment', default={})
diff --git a/src/buildstream/buildelement.py b/src/buildstream/buildelement.py
index b79876843..b33acfe12 100644
--- a/src/buildstream/buildelement.py
+++ b/src/buildstream/buildelement.py
@@ -281,7 +281,7 @@ class BuildElement(Element):
     #                   Private Local Methods                   #
     #############################################################
     def __get_commands(self, node, name):
-        raw_commands = node.get_sequence(name, []).as_str_list()
+        raw_commands = node.get_str_list(name, [])
         return [
             self.substitute_variables(command)
             for command in raw_commands
diff --git a/src/buildstream/element.py b/src/buildstream/element.py
index 5856f3241..af171621c 100644
--- a/src/buildstream/element.py
+++ b/src/buildstream/element.py
@@ -816,7 +816,7 @@ class Element(Plugin):
 
         if bstdata is not None:
             with sandbox.batch(SandboxFlags.NONE):
-                commands = bstdata.get_sequence('integration-commands', []).as_str_list()
+                commands = bstdata.get_str_list('integration-commands', [])
                 for command in commands:
                     cmd = self.substitute_variables(command)
 
@@ -2624,7 +2624,7 @@ class Element(Plugin):
         else:
             project_nocache = project.base_env_nocache
 
-        default_nocache = cls.__defaults.get_sequence('environment-nocache', default=[]).as_str_list()
+        default_nocache = cls.__defaults.get_str_list('environment-nocache', default=[])
         element_nocache = meta.env_nocache
 
         # Accumulate values from the element default, the project and the element
@@ -2866,7 +2866,7 @@ class Element(Plugin):
         # If this ever changes, things will go wrong unexpectedly.
         if not self.__whitelist_regex:
             bstdata = self.get_public_data('bst')
-            whitelist = bstdata.get_sequence('overlap-whitelist', default=[]).as_str_list()
+            whitelist = bstdata.get_str_list('overlap-whitelist', default=[])
             whitelist_expressions = [utils._glob2re(self.__variables.subst(exp.strip())) for exp in whitelist]
             expression = ('^(?:' + '|'.join(whitelist_expressions) + ')$')
             self.__whitelist_regex = re.compile(expression)
diff --git a/src/buildstream/node.pxd b/src/buildstream/node.pxd
index fdfa06c70..18520146d 100644
--- a/src/buildstream/node.pxd
+++ b/src/buildstream/node.pxd
@@ -52,6 +52,7 @@ cdef class MappingNode(Node):
     cpdef ScalarNode get_scalar(self, str key, default=*)
     cpdef SequenceNode get_sequence(self, str key, object default=*)
     cpdef str get_str(self, str key, object default=*)
+    cpdef list get_str_list(self, str key, object default=*)
     cpdef object items(self)
     cpdef list keys(self)
     cpdef void safe_del(self, str key)
diff --git a/src/buildstream/node.pyx b/src/buildstream/node.pyx
index aa1ff609d..fc17b8efa 100644
--- a/src/buildstream/node.pyx
+++ b/src/buildstream/node.pyx
@@ -679,6 +679,28 @@ cdef class MappingNode(Node):
         cdef ScalarNode scalar = self.get_scalar(key, default)
         return scalar.as_str()
 
+    cpdef list get_str_list(self, str key, object default=_sentinel):
+        """get_str_list(key, default=sentinel)
+
+        Get the value of the node for `key` as a list of strings.
+
+        This is equivalent to: :code:`mapping.get_sequence(my_key, my_default).as_str_list()`.
+
+        Args:
+            key (str): key for which to get the value
+            default (str): default value to return if `key` is not in the mapping
+
+        Raises:
+           :class:`buildstream._exceptions.LoadError`: if the value at `key` is not a
+                                                       :class:`.SequenceNode` or if any
+                                                       of its internal values is not a ScalarNode.
+
+        Returns:
+            :class:`list`: the value at `key` or the default
+        """
+        cdef SequenceNode sequence = self.get_sequence(key, default)
+        return sequence.as_str_list()
+
     cpdef object items(self):
         """Get a new view of the mapping items ((key, value) pairs).
 
diff --git a/src/buildstream/plugins/elements/compose.py b/src/buildstream/plugins/elements/compose.py
index 83501d817..1c523eeb2 100644
--- a/src/buildstream/plugins/elements/compose.py
+++ b/src/buildstream/plugins/elements/compose.py
@@ -66,8 +66,8 @@ class ComposeElement(Element):
         # We name this variable 'integration' only to avoid
         # collision with the Element.integrate() method.
         self.integration = node.get_bool('integrate')
-        self.include = node.get_sequence('include').as_str_list()
-        self.exclude = node.get_sequence('exclude').as_str_list()
+        self.include = node.get_str_list('include')
+        self.exclude = node.get_str_list('exclude')
         self.include_orphans = node.get_bool('include-orphans')
 
     def preflight(self):
diff --git a/src/buildstream/plugins/sources/pip.py b/src/buildstream/plugins/sources/pip.py
index 78c11fd89..40ddf8c68 100644
--- a/src/buildstream/plugins/sources/pip.py
+++ b/src/buildstream/plugins/sources/pip.py
@@ -114,8 +114,8 @@ class PipSource(Source):
         self.ref = node.get_str('ref', None)
         self.original_url = node.get_str('url', _PYPI_INDEX_URL)
         self.index_url = self.translate_url(self.original_url)
-        self.packages = node.get_sequence('packages', []).as_str_list()
-        self.requirements_files = node.get_sequence('requirements-files', []).as_str_list()
+        self.packages = node.get_str_list('packages', [])
+        self.requirements_files = node.get_str_list('requirements-files', [])
 
         if not (self.packages or self.requirements_files):
             raise SourceError("{}: Either 'packages' or 'requirements-files' must be specified". format(self))
diff --git a/tests/format/include.py b/tests/format/include.py
index 8902aa3eb..434a94d1f 100644
--- a/tests/format/include.py
+++ b/tests/format/include.py
@@ -197,7 +197,7 @@ def test_include_element_overrides_composition(cli, datafiles):
         'element.bst'])
     result.assert_success()
     loaded = _yaml.load_data(result.output)
-    assert loaded.get_sequence('build-commands').as_str_list() == ['first', 'second']
+    assert loaded.get_str_list('build-commands') == ['first', 'second']
 
 
 @pytest.mark.datafiles(DATA_DIR)
@@ -215,7 +215,7 @@ def test_list_overide_does_not_fail_upon_first_composition(cli, datafiles):
     # Assert that the explicitly overwritten public data is present
     bst = loaded.get_mapping('bst')
     assert 'foo-commands' in bst
-    assert bst.get_sequence('foo-commands').as_str_list() == ['need', 'this']
+    assert bst.get_str_list('foo-commands') == ['need', 'this']
 
 
 @pytest.mark.datafiles(DATA_DIR)
diff --git a/tests/format/include_composition.py b/tests/format/include_composition.py
index ec48d82a2..e10e28bc0 100644
--- a/tests/format/include_composition.py
+++ b/tests/format/include_composition.py
@@ -30,7 +30,7 @@ def test_main_has_priority(tmpdir):
 
         includes.process(main)
 
-        assert main.get_sequence('test').as_str_list() == ['main']
+        assert main.get_str_list('test') == ['main']
 
 
 def test_include_cannot_append(tmpdir):
@@ -45,7 +45,7 @@ def test_include_cannot_append(tmpdir):
 
         includes.process(main)
 
-        assert main.get_sequence('test').as_str_list() == ['main']
+        assert main.get_str_list('test') == ['main']
 
 
 def test_main_can_append(tmpdir):
@@ -59,7 +59,7 @@ def test_main_can_append(tmpdir):
 
         includes.process(main)
 
-        assert main.get_sequence('test').as_str_list() == ['a', 'main']
+        assert main.get_str_list('test') == ['a', 'main']
 
 
 def test_sibling_cannot_append_backward(tmpdir):
@@ -76,7 +76,7 @@ def test_sibling_cannot_append_backward(tmpdir):
 
         includes.process(main)
 
-        assert main.get_sequence('test').as_str_list() == ['b']
+        assert main.get_str_list('test') == ['b']
 
 
 def test_sibling_can_append_forward(tmpdir):
@@ -93,7 +93,7 @@ def test_sibling_can_append_forward(tmpdir):
 
         includes.process(main)
 
-        assert main.get_sequence('test').as_str_list() == ['a', 'b']
+        assert main.get_str_list('test') == ['a', 'b']
 
 
 def test_lastest_sibling_has_priority(tmpdir):
@@ -110,7 +110,7 @@ def test_lastest_sibling_has_priority(tmpdir):
 
         includes.process(main)
 
-        assert main.get_sequence('test').as_str_list() == ['b']
+        assert main.get_str_list('test') == ['b']
 
 
 def test_main_keeps_keys(tmpdir):
@@ -124,7 +124,7 @@ def test_main_keeps_keys(tmpdir):
 
         includes.process(main)
 
-        assert main.get_sequence('test').as_str_list() == ['a']
+        assert main.get_str_list('test') == ['a']
         assert main.get_str('something') == 'else'
 
 
@@ -147,5 +147,5 @@ def test_overwrite_directive_on_later_composite(tmpdir):
 
         includes.process(main)
 
-        assert main.get_sequence('test').as_str_list() == ['Overwritten']
+        assert main.get_str_list('test') == ['Overwritten']
         assert main.get_str('foo') == 'should be present'
diff --git a/tests/format/projectoverrides.py b/tests/format/projectoverrides.py
index 730e43b1e..7932ffb4a 100644
--- a/tests/format/projectoverrides.py
+++ b/tests/format/projectoverrides.py
@@ -24,6 +24,6 @@ def test_prepend_configure_commands(cli, datafiles):
 
     result.assert_success()
     loaded = _yaml.load_data(result.output)
-    config_commands = loaded.get_sequence('configure-commands').as_str_list()
+    config_commands = loaded.get_str_list('configure-commands')
     assert len(config_commands) == 3
     assert config_commands[0] == 'echo "Hello World!"'
diff --git a/tests/frontend/project/sources/fetch_source.py b/tests/frontend/project/sources/fetch_source.py
index ead5bc3f5..ac3020ec2 100644
--- a/tests/frontend/project/sources/fetch_source.py
+++ b/tests/frontend/project/sources/fetch_source.py
@@ -38,7 +38,7 @@ class FetchFetcher(SourceFetcher):
 class FetchSource(Source):
     # Read config to know which URLs to fetch
     def configure(self, node):
-        self.original_urls = node.get_sequence('urls').as_str_list()
+        self.original_urls = node.get_str_list('urls')
         self.output_file = node.get_str('output-text')
         self.fetch_succeeds = {
             key: value.as_bool()
diff --git a/tests/internals/yaml.py b/tests/internals/yaml.py
index d94d006e3..93619dc4c 100644
--- a/tests/internals/yaml.py
+++ b/tests/internals/yaml.py
@@ -146,7 +146,7 @@ def test_node_set_overwrite(datafiles):
     assert base.get_str('kind') == 'cow'
 
     # Overwrite a list as a string
-    assert base.get_sequence('moods').as_str_list() == ['happy', 'sad']
+    assert base.get_str_list('moods') == ['happy', 'sad']
     base['moods'] = 'unemotional'
     assert base.get_str('moods') == 'unemotional'
 
@@ -160,10 +160,10 @@ def test_node_set_list_element(datafiles):
 
     base = _yaml.load(filename)
 
-    assert base.get_sequence('moods').as_str_list() == ['happy', 'sad']
+    assert base.get_str_list('moods') == ['happy', 'sad']
     base.get_sequence('moods')[0] = 'confused'
 
-    assert base.get_sequence('moods').as_str_list() == ['confused', 'sad']
+    assert base.get_str_list('moods') == ['confused', 'sad']
 
 
 # Really this is testing _yaml.node_copy(), we want to