Re-architect the RepoFlags class

We don't need to create flags - just read and parse them from
the project meta config. Remove the unneeded functionality and
enhance the structure. All flags from a project and package
are accessible via a single RepoFlags object.

4 files changed, 53 insertions, 168 deletions

diff --git a/obsapi/core.py b/obsapi/core.py
index 3bc1e3c..93de12f 100644
--- a/obsapi/core.py
+++ b/obsapi/core.py
@@ -299,8 +299,8 @@ class ObsApi(object):
 
         return vendor
 
-    def get_repo_flags(self, flag_type, prj, pkg=None):
-        return RepoFlags(flag_type, self.get_meta(prj, pkg))
+    def get_repo_flags(self, prj, pkg=None):
+        return RepoFlags(self.get_meta(prj, pkg))
 
     def locked(self, prj, pkg=None):
         meta = self.__xml2etree(self.get_meta(prj, pkg))
diff --git a/obsapi/repoflags.py b/obsapi/repoflags.py
index 3689711..c7badc7 100644
--- a/obsapi/repoflags.py
+++ b/obsapi/repoflags.py
@@ -3,7 +3,7 @@ from lxml import etree
 from collections import namedtuple
 
 
-RepoFlag = namedtuple('RepoFlag', 'status repository arch')
+RepoFlag = namedtuple('RepoFlag', 'flag status repository arch')
 valid_flag_types = ('build', 'publish', 'useforbuild', 'debuginfo')
 
 if sys.version_info >= (3, 0):
@@ -12,9 +12,8 @@ if sys.version_info >= (3, 0):
 
 class RepoFlags():
 
-    def __init__(self, flag_type=None, xml=None):
+    def __init__(self, xml):
 
-        self.flag_type = flag_type
         self.root = None
 
         if isinstance(xml, (str, unicode, bytes)):
@@ -25,10 +24,19 @@ class RepoFlags():
         elif isinstance(xml, etree._Element):
             self.root = xml
 
-        if self.flag_type is None and self.root is None:
-            self.flag_type = 'build'
+    def __str__(self):
+        lines = []
 
-        self.__verify()
+        for flag_type in self.flag_types:
+            lines.append('{}:'.format(flag_type))
+            for flag in self.get_flag(flag_type):
+                line = ''
+                for attribute in [flag.status, flag.repository, flag.arch]:
+                    if attribute is not None:
+                        line = ' '.join([line, attribute])
+                lines.append(line)
+
+        return '\n'.join(lines)
 
     def import_xml(self, xml):
         try:
@@ -36,72 +44,20 @@ class RepoFlags():
         except Exception as e:
             raise e
 
-    def __verify(self):
-        """ Make sure flag_type and xml data match.
-        """
-
-        # if self.flag_type set, ensure it matches the tag of the root xml
-        # element. If not matching, try to find the matching element within
-        # the xml tree and set root to that. If not found, set root to a new
-        # empty tree based on the flag_type
-        if self.flag_type is not None:
-            if self.flag_type not in valid_flag_types:
-                raise ValueError('Invalid repo flag type %s' % self.flag_type)
-            if self.root is None:
-                self.root = etree.Element(self.flag_type)
-                return
-            if self.flag_type == self.root.tag:
-                return
-
-            element = self.root.find(self.flag_type)
-            if element is not None:
-                self.root = element
-                return
-
-            # element matching flag_type not found in xml, create new element
-            self.root = etree.Element(self.flag_type)
-            return
-        else:
-            # If flag_type is not set, set it to the root element tag
-            flag_type = self.root.tag
-            if flag_type in valid_flag_types:
-                return
-
-        # if root element is not a valid repo flag type, search for the
-        # first element that is a valid flag type and use it.
-        for flag_type in valid_flag_types:
-            element = self.root.find(flag_type)
-            if element is not None:
-                self.flag_type = flag_type
-                self.root = element
-                return
-
-        self.flag_type = 'build'
-        self.root = etree.Element(self.flag_type)
-
     @property
     def flags(self):
         for element in self.root:
-            if element.tag not in ['enable', 'disable']:
-                continue
-            status = element.tag
-            repo = element.get('repository', None)
-            arch = element.get('arch', None)
-            yield(RepoFlag(status=status, repository=repo, arch=arch))
-
-    def set(self, status, repo=None, arch=None):
-        if status not in ['enable', 'disable']:
-            raise ValueError('Invalid Flag status %s' % status)
+            for sub in element:
+                if sub.tag in ['enable', 'disable']:
+                    flag = element.tag
+                    status = sub.tag
+                    repo = sub.get('repository', None)
+                    arch = sub.get('arch', None)
+                    yield(RepoFlag(flag=flag, status=status, repository=repo, arch=arch))
 
-        element = etree.Element(status)
-        if repo:
-            element.set('repository', repo)
-        if arch:
-            element.set('arch', arch)
-        self.root.append(element)
+    def get_flag(self, flag_type):
+        return [f for f in self.flags if f.flag == flag_type]
 
-    def enable(self, repo=None, arch=None):
-        self.set('enable', repo, arch)
-
-    def disable(self, repo=None, arch=None):
-        self.set('disable', repo, arch)
+    @property
+    def flag_types(self):
+        return set([f.flag for f in self.flags])
diff --git a/test/test_api.py b/test/test_api.py
index c8e40b2..5769543 100644
--- a/test/test_api.py
+++ b/test/test_api.py
@@ -357,24 +357,31 @@ def test_get_binary_fileinfo(requests_mock, binary_fileinfo):
     assert bfinfo.requires == [p.text for p in expected_bfinfo.findall('requires')]
 
 
-def check_get_repo_flags(flag_type, prj, pkg, expected):
-    repo_flags = api.get_repo_flags(flag_type, prj, pkg)
-    assert repo_flags.flag_type == flag_type
-    assert len(list(repo_flags.flags)) == expected
-
-
-def test_get_project_repo_build_flags(requests_mock, project_meta):
+def test_get_project_flags(requests_mock, project_meta):
     matcher = prj_meta_re
     requests_mock.get(matcher, text=project_meta)
-    check_get_repo_flags('build', prj, None, 0)
+    repoflags = api.get_repo_flags(prj)
+    assert repoflags.flag_types == set()
 
 
-def test_get_package_repo_build_flags(requests_mock, package_meta):
+def test_get_package_repo_flags(requests_mock, package_meta):
     matcher = pkg_meta_re
     requests_mock.get(matcher, text=package_meta)
+    repoflags = api.get_repo_flags(prj, pkg)
+    assert repoflags.flag_types == {'build', 'debuginfo', 'useforbuild'}
     meta = etree.fromstring(package_meta)
-    expected_repoflags = meta.find('build')
-    check_get_repo_flags('build', prj, pkg, len(expected_repoflags))
+    for element in meta:
+        for sub in element:
+            if sub.tag in ['enable', 'disable']:
+                flag = element.tag
+                status = sub.tag
+                repository = sub.get('repository', None)
+                arch = sub.get('arch', None)
+                match = False
+                for flag in repoflags.get_flag(flag):
+                    if flag.status == status and flag.repository == repository and flag.arch == arch:
+                        match = True
+                assert match
 
 
 @pytest.mark.parametrize('target', ('project', 'package'))
diff --git a/test/test_repo_flags.py b/test/test_repo_flags.py
index 8219eef..d9b8642 100644
--- a/test/test_repo_flags.py
+++ b/test/test_repo_flags.py
@@ -34,88 +34,10 @@ xml2 = """<package name="suse-hello-1.0" project="home:sbahling:obsapi:test">
 flag_types = ['build', 'publish', 'useforbuild', 'debuginfo']
 
 
-def test_repo_flags_default_type():
-    repo_flags = RepoFlags()
-    assert repo_flags.flag_type == 'build'
-
-
-@pytest.mark.parametrize('flag_type', flag_types)
-def test_repo_flags_flag_type(flag_type):
-    repo_flags = RepoFlags(flag_type)
-    assert repo_flags.flag_type == flag_type
-    assert repo_flags.root.tag == flag_type
-
-
-def test_repo_flags_invalid_flag_type():
-    with pytest.raises(ValueError):
-        RepoFlags('invalid')
-
-
-def test_repo_flags_pass_xml_without_flags():
-    repo_flags = RepoFlags(xml=xml1)
-    assert repo_flags.flag_type == 'build'
-
-
-def test_repo_flags_pass_xml_with_flags():
-    repo_flags = RepoFlags(xml=xml2)
-    assert repo_flags.flag_type in flag_types
-
-
-def test_repo_flags_xml_build_flags():
-    repo_flags = RepoFlags('build', xml=xml2)
-    assert repo_flags.flag_type == 'build'
-    for flag in repo_flags.flags:
-        if flag.status == 'enable':
-            assert flag.repository == 'SLE_12_SP3'
-            assert flag.arch == 'x86_64'
-        if flag.status == 'disable':
-            assert flag.repository is None
-            assert flag.arch is None
-
-
-def test_repo_flags_xml_debuginfo_flags():
-    repo_flags = RepoFlags('debuginfo', xml=xml2)
-    assert repo_flags.flag_type == 'debuginfo'
-    for flag in repo_flags.flags:
-        if flag.status == 'enable':
-            assert flag.repository is None
-            assert flag.arch is None
-
-
-def test_repo_flags_xml_new_flags():
-    repo_flags = RepoFlags('publish', xml=xml2)
-    assert repo_flags.flag_type == 'publish'
-    assert len(list(repo_flags.flags)) == 0
-
-
-params = [('useforbuild', 'SLE_15_SP1', 'x86_64'),
-          ('publish', None, None),
-          ('debuginfo', None, 'aarch64'),
-          ('build', 'SLE_12_SP3', None)
-          ]
-
-
-@pytest.mark.parametrize('flag_type, repo, arch', params)
-def test_repo_flags_enable(flag_type, repo, arch):
-    repoflags = RepoFlags(flag_type)
-    repoflags.enable(repo=repo, arch=arch)
-    for flag in repoflags.flags:
-        assert flag.status == 'enable'
-        assert flag.repository == repo
-        assert flag.arch == arch
-
-
-@pytest.mark.parametrize('flag_type, repo, arch', params)
-def test_repo_flags_disable(flag_type, repo, arch):
-    repoflags = RepoFlags(flag_type)
-    repoflags.disable(repo=repo, arch=arch)
-    for flag in repoflags.flags:
-        assert flag.status == 'disable'
-        assert flag.repository == repo
-        assert flag.arch == arch
-
-
-def test_repo_flags_set_invalid():
-    repoflags = RepoFlags()
-    with pytest.raises(ValueError):
-        repoflags.set('invalid')
+def test_flag_types():
+    repoflags = RepoFlags(xml2)
+    flag_types = repoflags.flag_types
+    assert len(flag_types) == 3
+    assert 'build' in flag_types
+    assert 'useforbuild' in flag_types
+    assert 'debuginfo' in flag_types