Refactor environment functions in preparation for get_file

These functions have been reorganised to break out functions that
deal with only file or URL handling to allow for better unit testing
and to prepare for get_file which will also use some of these functions.

Change-Id: I8576d09c556fc0835004da6b78070686482361e0

3 files changed, 226 insertions, 95 deletions

diff --git a/heatclient/common/template_utils.py b/heatclient/common/template_utils.py
index 6eecb8a..7d077fa 100644
--- a/heatclient/common/template_utils.py
+++ b/heatclient/common/template_utils.py
@@ -15,8 +15,8 @@
 
 import os
 import urllib
-import yaml
 
+from heatclient.common import environment_format
 from heatclient.common import template_format
 from heatclient import exc
 from heatclient.openstack.common.py3kcompat import urlutils
@@ -27,8 +27,7 @@ def get_template_contents(template_file=None, template_url=None,
 
     # Transform a bare file path to a file:// URL.
     if template_file:
-        template_url = urlutils.urljoin(
-            'file:', urllib.pathname2url(template_file))
+        template_url = normalise_file_path_to_url(template_file)
 
     if template_url:
         tpl = urlutils.urlopen(template_url).read()
@@ -53,50 +52,60 @@ def get_template_contents(template_file=None, template_url=None,
             'Error parsing template %s %s' % (template_url, e))
 
 
-def get_file_contents(resource_registry, fields, base_url='',
+def get_file_contents(from_dict, files, base_url=None,
                       ignore_if=None):
-    for key, value in iter(resource_registry.items()):
+    for key, value in iter(from_dict.items()):
         if ignore_if and ignore_if(key, value):
             continue
 
-        if base_url != '' and not base_url.endswith('/'):
+        if base_url and not base_url.endswith('/'):
             base_url = base_url + '/'
+
         str_url = urlutils.urljoin(base_url, value)
         try:
-            fields['files'][str_url] = urlutils.urlopen(str_url).read()
+            files[str_url] = urlutils.urlopen(str_url).read()
         except urlutils.URLError:
             raise exc.CommandError('Could not fetch %s from the environment'
                                    % str_url)
-        resource_registry[key] = str_url
+        from_dict[key] = str_url
+
+
+def base_url_for_url(url):
+    parsed = urlutils.urlparse(url)
+    parsed_dir = os.path.dirname(parsed.path)
+    return urlutils.urljoin(url, parsed_dir)
+
 
+def normalise_file_path_to_url(path):
+    if urlutils.urlparse(path).scheme:
+        return path
+    path = os.path.abspath(path)
+    return urlutils.urljoin('file:', urllib.pathname2url(path))
 
-def prepare_environment(env_path):
-    if (not urlutils.urlparse(env_path).scheme):
-        env_path = urlutils.urljoin(
-            'file:', urllib.pathname2url(env_path))
-    raw_env = urlutils.urlopen(env_path).read()
-    env = yaml.safe_load(raw_env)
-    remote = urlutils.urlparse(env_path)
-    remote_dir = os.path.dirname(remote.path)
-    environment_base_url = urlutils.urljoin(env_path, remote_dir)
-    return environment_base_url, env
 
+def process_environment_and_files(env_path=None, template_path=None):
+    files = {}
+    env = {}
 
-def process_environment_and_files(fields, env_path):
     if not env_path:
-        return
+        return files, None
 
-    environment_url, env = prepare_environment(env_path)
+    env_url = normalise_file_path_to_url(env_path)
+    env_base_url = base_url_for_url(env_url)
+    raw_env = urlutils.urlopen(env_url).read()
+    env = environment_format.parse(raw_env)
 
-    fields['environment'] = env
-    fields['files'] = {}
+    resolve_environment_urls(
+        env.get('resource_registry'),
+        files,
+        env_base_url)
 
-    resolve_environment_urls(fields, environment_url)
+    return files, env
 
 
-def resolve_environment_urls(fields, environment_url):
-    rr = fields['environment'].get('resource_registry', {})
-    base_url = rr.get('base_url', environment_url)
+def resolve_environment_urls(resource_registry, files, env_base_url):
+    rr = resource_registry
+    base_url = rr.get('base_url', env_base_url)
 
     def ignore_if(key, value):
         if key == 'base_url':
@@ -108,8 +117,8 @@ def resolve_environment_urls(fields, environment_url):
             # don't need downloading.
             return True
 
-    get_file_contents(rr, fields, base_url, ignore_if)
+    get_file_contents(rr, files, base_url, ignore_if)
 
     for res_name, res_dict in iter(rr.get('resources', {}).items()):
         res_base_url = res_dict.get('base_url', base_url)
-        get_file_contents(res_dict, fields, res_base_url, ignore_if)
+        get_file_contents(res_dict, files, res_base_url, ignore_if)
diff --git a/heatclient/tests/test_template_utils.py b/heatclient/tests/test_template_utils.py
index ed703f8..d66b511 100644
--- a/heatclient/tests/test_template_utils.py
+++ b/heatclient/tests/test_template_utils.py
@@ -36,85 +36,135 @@ class ShellEnvironmentTest(testtools.TestCase):
     def collect_links(self, env, content, url, env_base_url=''):
 
         jenv = yaml.safe_load(env)
-        fields = {
-            'files': {},
-            'environment': jenv
-        }
+        files = {}
         if url:
             self.m.StubOutWithMock(urlutils, 'urlopen')
             urlutils.urlopen(url).AndReturn(six.StringIO(content))
             self.m.ReplayAll()
 
-        template_utils.resolve_environment_urls(fields, env_base_url)
+        template_utils.resolve_environment_urls(
+            jenv.get('resource_registry'), files, env_base_url)
         if url:
-            self.assertEqual(fields['files'][url], content)
-
-    def test_prepare_environment_file(self):
-        with tempfile.NamedTemporaryFile() as env_file:
-            env = '''
-            resource_registry:
-              "OS::Thingy": "file:///home/b/a.yaml"
-            '''
-            env_file.write(env)
-            env_file.flush()
-            env_url, env_dict = template_utils.prepare_environment(
-                env_file.name)
-            self.assertEqual(
-                {'resource_registry': {'OS::Thingy': 'file:///home/b/a.yaml'}},
-                env_dict)
-            env_dir = os.path.dirname(env_file.name)
-            self.assertEqual(env_url, 'file://%s' % env_dir)
-
-    def test_prepare_environment_relative_file(self):
-        with tempfile.NamedTemporaryFile() as env_file:
-            env = '''
-            resource_registry:
-              "OS::Thingy": a.yaml
-            '''
-            env_file.write(env)
-            env_file.flush()
-            env_url, env_dict = template_utils.prepare_environment(
-                env_file.name)
-            self.assertEqual(
-                {'resource_registry': {'OS::Thingy': 'a.yaml'}},
-                env_dict)
-            env_dir = os.path.dirname(env_file.name)
-            self.assertEqual(env_url, 'file://%s' % env_dir)
+            self.assertEqual(files[url], content)
 
-    def test_prepare_environment_url(self):
+    def test_process_environment_file(self):
+
+        self.m.StubOutWithMock(urlutils, 'urlopen')
+        env_file = '/home/my/dir/env.yaml'
         env = '''
         resource_registry:
-            "OS::Thingy": "a.yaml"
+          "OS::Thingy": "file:///home/b/a.yaml"
         '''
-        url = 'http://no.where/some/path/to/file.yaml'
+        tmpl = '{"foo": "bar"}'
+
+        urlutils.urlopen('file://%s' % env_file).AndReturn(
+            six.StringIO(env))
+        urlutils.urlopen('file:///home/b/a.yaml').AndReturn(
+            six.StringIO(tmpl))
+        self.m.ReplayAll()
+
+        files, env_dict = template_utils.process_environment_and_files(
+            env_file)
+        self.assertEqual(
+            {'resource_registry': {
+                'OS::Thingy': 'file:///home/b/a.yaml'}},
+            env_dict)
+        self.assertEqual('{"foo": "bar"}', files['file:///home/b/a.yaml'])
+
+    def test_process_environment_relative_file(self):
+
         self.m.StubOutWithMock(urlutils, 'urlopen')
-        urlutils.urlopen(url).AndReturn(six.StringIO(env))
+        env_file = '/home/my/dir/env.yaml'
+        env_url = 'file:///home/my/dir/env.yaml'
+        env = '''
+        resource_registry:
+          "OS::Thingy": a.yaml
+        '''
+        tmpl = '{"foo": "bar"}'
+
+        urlutils.urlopen(env_url).AndReturn(
+            six.StringIO(env))
+        urlutils.urlopen('file:///home/my/dir/a.yaml').AndReturn(
+            six.StringIO(tmpl))
         self.m.ReplayAll()
-        env_url, env_dict = template_utils.prepare_environment(url)
-        self.assertEqual({'resource_registry': {'OS::Thingy': 'a.yaml'}},
-                         env_dict)
-        self.assertEqual('http://no.where/some/path/to', env_url)
 
-    def test_process_environment_and_files(self):
+        self.assertEqual(
+            env_url,
+            template_utils.normalise_file_path_to_url(env_file))
+        self.assertEqual(
+            'file:///home/my/dir',
+            template_utils.base_url_for_url(env_url))
+
+        files, env_dict = template_utils.process_environment_and_files(
+            env_file)
+
+        self.assertEqual(
+            {'resource_registry': {
+                'OS::Thingy': 'file:///home/my/dir/a.yaml'}},
+            env_dict)
+        self.assertEqual(
+            '{"foo": "bar"}', files['file:///home/my/dir/a.yaml'])
+
+    def test_process_environment_relative_file_up(self):
+
+        self.m.StubOutWithMock(urlutils, 'urlopen')
+        env_file = '/home/my/dir/env.yaml'
+        env_url = 'file:///home/my/dir/env.yaml'
+        env = '''
+        resource_registry:
+          "OS::Thingy": ../bar/a.yaml
+        '''
+        tmpl = '{"foo": "bar"}'
+
+        urlutils.urlopen(env_url).AndReturn(
+            six.StringIO(env))
+        urlutils.urlopen('file:///home/my/bar/a.yaml').AndReturn(
+            six.StringIO(tmpl))
+        self.m.ReplayAll()
+
+        env_url = 'file://%s' % env_file
+        self.assertEqual(
+            env_url,
+            template_utils.normalise_file_path_to_url(env_file))
+        self.assertEqual(
+            'file:///home/my/dir',
+            template_utils.base_url_for_url(env_url))
+
+        files, env_dict = template_utils.process_environment_and_files(
+            env_file)
+
+        self.assertEqual(
+            {'resource_registry': {
+                'OS::Thingy': 'file:///home/my/bar/a.yaml'}},
+            env_dict)
+        self.assertEqual(
+            '{"foo": "bar"}', files['file:///home/my/bar/a.yaml'])
+
+    def test_process_environment_url(self):
         env = '''
         resource_registry:
             "OS::Thingy": "a.yaml"
         '''
         url = 'http://no.where/some/path/to/file.yaml'
-        a_url = 'http://no.where/some/path/to/a.yaml'
+        tmpl_url = 'http://no.where/some/path/to/a.yaml'
+        tmpl = '{"foo": "bar"}'
+
         self.m.StubOutWithMock(urlutils, 'urlopen')
         urlutils.urlopen(url).AndReturn(six.StringIO(env))
-        urlutils.urlopen(a_url).AndReturn(six.StringIO("A's contents."))
-
+        urlutils.urlopen(tmpl_url).AndReturn(six.StringIO(tmpl))
         self.m.ReplayAll()
-        fields = {}
-        template_utils.process_environment_and_files(fields, url)
-        self.assertEqual("A's contents.", fields['files'][a_url])
+
+        files, env_dict = template_utils.process_environment_and_files(
+            url)
+
+        self.assertEqual({'resource_registry': {'OS::Thingy': tmpl_url}},
+                         env_dict)
+        self.assertEqual(tmpl, files[tmpl_url])
 
     def test_no_process_environment_and_files(self):
-        fields = {}
-        template_utils.process_environment_and_files(fields, None)
-        self.assertEqual({}, fields)
+        files, env = template_utils.process_environment_and_files()
+        self.assertIsNone(env)
+        self.assertEqual({}, files)
 
     def test_global_files(self):
         a = "A's contents."
@@ -288,3 +338,68 @@ class TestGetTemplateContents(testtools.TestCase):
 
         self.assertEqual({"foo": "bar"}, tmpl_parsed)
         self.assertTrue(self.object_requested)
+
+
+class TestURLFunctions(testtools.TestCase):
+
+    def setUp(self):
+        super(TestURLFunctions, self).setUp()
+        self.m = mox.Mox()
+
+        self.addCleanup(self.m.VerifyAll)
+        self.addCleanup(self.m.UnsetStubs)
+
+    def test_normalise_file_path_to_url_relative(self):
+        self.assertEqual(
+            'file://%s/foo' % os.getcwd(),
+            template_utils.normalise_file_path_to_url(
+                'foo'))
+
+    def test_normalise_file_path_to_url_absolute(self):
+        self.assertEqual(
+            'file:///tmp/foo',
+            template_utils.normalise_file_path_to_url(
+                '/tmp/foo'))
+
+    def test_normalise_file_path_to_url_file(self):
+        self.assertEqual(
+            'file:///tmp/foo',
+            template_utils.normalise_file_path_to_url(
+                'file:///tmp/foo'))
+
+    def test_normalise_file_path_to_url_http(self):
+        self.assertEqual(
+            'http://localhost/foo',
+            template_utils.normalise_file_path_to_url(
+                'http://localhost/foo'))
+
+    def test_base_url_for_url(self):
+        self.assertEqual(
+            'file:///foo/bar',
+            template_utils.base_url_for_url(
+                'file:///foo/bar/baz'))
+        self.assertEqual(
+            'file:///foo/bar',
+            template_utils.base_url_for_url(
+                'file:///foo/bar/baz.txt'))
+        self.assertEqual(
+            'file:///foo/bar',
+            template_utils.base_url_for_url(
+                'file:///foo/bar/'))
+        self.assertEqual(
+            'file:///',
+            template_utils.base_url_for_url(
+                'file:///'))
+        self.assertEqual(
+            'file:///',
+            template_utils.base_url_for_url(
+                'file:///foo'))
+
+        self.assertEqual(
+            'http://foo/bar',
+            template_utils.base_url_for_url(
+                'http://foo/bar/'))
+        self.assertEqual(
+            'http://foo/bar',
+            template_utils.base_url_for_url(
+                'http://foo/bar/baz.template'))
diff --git a/heatclient/v1/shell.py b/heatclient/v1/shell.py
index 10a941f..690a4db 100644
--- a/heatclient/v1/shell.py
+++ b/heatclient/v1/shell.py
@@ -69,6 +69,9 @@ def do_create(hc, args):
            help='Name of the stack to create.')
 def do_stack_create(hc, args):
     '''Create the stack.'''
+    files, env = template_utils.process_environment_and_files(
+        env_path=args.environment_file)
+
     fields = {
         'stack_name': args.name,
         'timeout_mins': args.create_timeout,
@@ -78,12 +81,11 @@ def do_stack_create(hc, args):
             args.template_file,
             args.template_url,
             args.template_object,
-            hc.http_client.raw_request)
+            hc.http_client.raw_request),
+        'files': files,
+        'environment': env
     }
 
-    template_utils.process_environment_and_files(fields,
-                                                 args.environment_file)
-
     hc.stacks.create(**fields)
     do_stack_list(hc)
 
@@ -204,6 +206,9 @@ def do_update(hc, args):
            help='Name or ID of stack to update.')
 def do_stack_update(hc, args):
     '''Update the stack.'''
+    files, env = template_utils.process_environment_and_files(
+        env_path=args.environment_file)
+
     fields = {
         'stack_id': args.id,
         'parameters': utils.format_parameters(args.parameters),
@@ -211,10 +216,10 @@ def do_stack_update(hc, args):
             args.template_file,
             args.template_url,
             args.template_object,
-            hc.http_client.raw_request)
+            hc.http_client.raw_request),
+        'files': files,
+        'environment': env
     }
-    template_utils.process_environment_and_files(fields,
-                                                 args.environment_file)
 
     hc.stacks.update(**fields)
     do_list(hc)
@@ -309,16 +314,18 @@ def do_validate(hc, args):
            action='append')
 def do_template_validate(hc, args):
     '''Validate a template with parameters.'''
+    files, env = template_utils.process_environment_and_files(
+        env_path=args.environment_file)
     fields = {
         'parameters': utils.format_parameters(args.parameters),
         'template': template_utils.get_template_contents(
             args.template_file,
             args.template_url,
             args.template_object,
-            hc.http_client.raw_request)
+            hc.http_client.raw_request),
+        'files': files,
+        'environment': env
     }
-    template_utils.process_environment_and_files(fields,
-                                                 args.environment_file)
 
     validation = hc.stacks.validate(**fields)
     print(jsonutils.dumps(validation, indent=2))