From bfecb5c86bb6e80e8cd89b2ab87952024bd801f9 Mon Sep 17 00:00:00 2001
From: Tiago Gomes <tiago.gomes@codethink.co.uk>
Date: Thu, 12 Nov 2015 15:48:07 +0000
Subject: morphloader: add missing validation to stratum chunks

Change-Id: I7268b732ab0e37a198cd0484f491334ff456de2b
---
 morphlib/morphloader.py       | 54 ++++++++++++++++++++-----------------------
 morphlib/morphloader_tests.py | 48 ++++++++++++++++++++++++++++++++------
 2 files changed, 66 insertions(+), 36 deletions(-)

diff --git a/morphlib/morphloader.py b/morphlib/morphloader.py
index 0160e419..3e735a41 100644
--- a/morphlib/morphloader.py
+++ b/morphlib/morphloader.py
@@ -59,6 +59,17 @@ class MissingFieldError(MorphologyValidationError):
             'Missing field %s from morphology %s' % (field, morphology_name))
 
 
+class InvalidStringError(MorphologyValidationError):
+
+    def __init__(self, field, spec, morph_filename):
+        self.field = field
+        self.spec = spec
+        self.morph_filename = morph_filename
+        MorphologyValidationError.__init__(
+            self, "Field '%(field)s' must be a non-empty string in %(spec)s"\
+                  " for morphology %(morph_filename)s" % locals())
+
+
 class InvalidFieldError(MorphologyValidationError):
 
     def __init__(self, field, morphology_name):
@@ -127,27 +138,6 @@ class DuplicateChunkError(MorphologyValidationError):
                   'in stratum %(stratum_name)s' % locals())
 
 
-class EmptyRefError(MorphologyValidationError):
-
-    def __init__(self, ref_location, morph_filename):
-        self.ref_location = ref_location
-        self.morph_filename = morph_filename
-        MorphologyValidationError.__init__(
-            self, 'Empty ref found for %(ref_location)s '\
-                  'in %(morph_filename)s' % locals())
-
-
-class ChunkSpecRefNotStringError(MorphologyValidationError):
-
-    def __init__(self, ref_value, chunk_name, stratum_name):
-        self.ref_value = ref_value
-        self.chunk_name = chunk_name
-        self.stratum_name = stratum_name
-        MorphologyValidationError.__init__(
-            self, 'Ref %(ref_value)s for %(chunk_name)s '\
-                  'in stratum %(stratum_name)s is not a string' % locals())
-
-
 class ChunkSpecConflictingFieldsError(MorphologyValidationError):
 
     def __init__(self, fields, chunk_name, stratum_name):
@@ -547,14 +537,20 @@ class MorphologyLoader(object):
         for spec in morph['chunks']:
             chunk_name = spec['name']
 
-            # All chunk refs must be strings.
-            if 'ref' in spec:
-                ref = spec['ref']
-                if ref == None:
-                    raise EmptyRefError(spec['name'], morph.filename)
-                elif not isinstance(ref, basestring):
-                    raise ChunkSpecRefNotStringError(
-                        ref, spec['name'], morph.filename)
+            # All chunks repos and refs must be strings
+
+            def validate_chunk_str_field(field, spec, morph_filename):
+                if field not in spec:
+                    raise MissingFieldError('%s in %s' % (field, spec),
+                                            morph.filename)
+                val = spec[field]
+                if not val or not isinstance(val, basestring) or (
+                        not val.strip()):
+                   raise InvalidStringError(
+                        field, spec, morph_filename)
+
+            validate_chunk_str_field('repo', spec, morph.filename)
+            validate_chunk_str_field('ref', spec, morph.filename)
 
             # The build-depends field must be a list.
             if 'build-depends' in spec:
diff --git a/morphlib/morphloader_tests.py b/morphlib/morphloader_tests.py
index 0bf69612..5f7d3915 100644
--- a/morphlib/morphloader_tests.py
+++ b/morphlib/morphloader_tests.py
@@ -208,7 +208,42 @@ build-system: manual
         self.assertRaises(
             morphlib.morphloader.InvalidFieldError, self.loader.validate, m)
 
-    def test_validate_requires_chunk_refs_in_stratum_to_be_strings(self):
+    def test_validate_requires_chunk_repos_to_exist(self):
+        m = morphlib.morphology.Morphology({
+            'kind': 'stratum',
+            'name': 'foo',
+            'build-depends': [],
+            'chunks': [
+                {
+                    'name': 'chunk',
+                    'ref': 'master',
+                    'build-depends': []
+                }
+            ]
+        })
+        with self.assertRaises(
+                morphlib.morphloader.MissingFieldError):
+            self.loader.validate(m)
+
+    def test_validate_requires_chunk_repos_in_stratum_to_be_strings(self):
+        m = morphlib.morphology.Morphology({
+            'kind': 'stratum',
+            'name': 'foo',
+            'build-depends': [],
+            'chunks': [
+                {
+                    'name': 'chunk',
+                    'repo': 1,
+                    'ref': 'master',
+                    'build-depends': []
+                }
+            ]
+        })
+        with self.assertRaises(
+                morphlib.morphloader.InvalidStringError):
+            self.loader.validate(m)
+
+    def test_validate_requires_chunk_refs_to_exist(self):
         m = morphlib.morphology.Morphology({
             'kind': 'stratum',
             'name': 'foo',
@@ -217,31 +252,30 @@ build-system: manual
                 {
                     'name': 'chunk',
                     'repo': 'test:repo',
-                    'ref': 1,
                     'build-depends': []
                 }
             ]
         })
         with self.assertRaises(
-                morphlib.morphloader.ChunkSpecRefNotStringError):
+                morphlib.morphloader.MissingFieldError):
             self.loader.validate(m)
 
-    def test_fails_to_validate_stratum_with_empty_refs_for_a_chunk(self):
+    def test_validate_requires_chunk_refs_in_stratum_to_be_strings(self):
         m = morphlib.morphology.Morphology({
             'kind': 'stratum',
             'name': 'foo',
             'build-depends': [],
-            'chunks' : [
+            'chunks': [
                 {
                     'name': 'chunk',
                     'repo': 'test:repo',
-                    'ref': None,
+                    'ref': 1,
                     'build-depends': []
                 }
             ]
         })
         with self.assertRaises(
-                morphlib.morphloader.EmptyRefError):
+                morphlib.morphloader.InvalidStringError):
             self.loader.validate(m)
 
     def test_fails_to_validate_stratum_with_a_missing_path(self):
-- 
cgit v1.2.1