Rename ServiceLoader to ServiceExtendsResolver

ServiceLoader has evolved to be not really all that related to "loading" a
service. It's responsibility is more to do with handling the `extends`
field, which is only part of loading.  The class and its primary method
(make_service_dict()) were renamed to better reflect their responsibility.

As part of that change process_container_options() was removed from
make_service_dict() and renamed to process_service().  It contains logic for
handling the non-extends options.

This change allows us to remove the hacks from testcase.py and only call
the functions we need to format a service dict correctly for integration tests.

Signed-off-by: Daniel Nephin <dnephin@docker.com>

compose/config/config.py

@@ -177,12 +177,12 @@ def load(config_details):
     """
 
     def build_service(filename, service_name, service_dict):
-        loader = ServiceLoader(
+        resolver = ServiceExtendsResolver(
             config_details.working_dir,
             filename,
             service_name,
             service_dict)
-        service_dict = loader.make_service_dict()
+        service_dict = process_service(config_details.working_dir, resolver.run())
         validate_paths(service_dict)
         return service_dict
 
@@ -224,7 +224,7 @@ def process_config_file(config_file, service_name=None):
     return config_file._replace(config=processed_config)
 
 
-class ServiceLoader(object):
+class ServiceExtendsResolver(object):
     def __init__(
         self,
         working_dir,
@@ -234,7 +234,7 @@ class ServiceLoader(object):
         already_seen=None
     ):
         if working_dir is None:
-            raise ValueError("No working_dir passed to ServiceLoader()")
+            raise ValueError("No working_dir passed to ServiceExtendsResolver()")
 
         self.working_dir = os.path.abspath(working_dir)
 
@@ -251,7 +251,7 @@ class ServiceLoader(object):
         if self.signature(name) in self.already_seen:
             raise CircularReference(self.already_seen + [self.signature(name)])
 
-    def make_service_dict(self):
+    def run(self):
         service_dict = dict(self.service_dict)
         env = resolve_environment(self.working_dir, self.service_dict)
         if env:
@@ -264,7 +264,7 @@ class ServiceLoader(object):
         if not self.already_seen:
             validate_against_service_schema(service_dict, self.service_name)
 
-        return process_container_options(self.working_dir, service_dict)
+        return service_dict
 
     def validate_and_construct_extends(self):
         extends = self.service_dict['extends']
@@ -281,19 +281,16 @@ class ServiceLoader(object):
         return config_path, service_config, service_name
 
     def resolve_extends(self, extended_config_path, service_config, service_name):
-        other_working_dir = os.path.dirname(extended_config_path)
+        resolver = ServiceExtendsResolver(
-        other_already_seen = self.already_seen + [self.signature(self.service_name)]
+            os.path.dirname(extended_config_path),
-
-        other_loader = ServiceLoader(
-            other_working_dir,
             extended_config_path,
             self.service_name,
             service_config,
-            already_seen=other_already_seen,
+            already_seen=self.already_seen + [self.signature(self.service_name)],
         )
 
-        other_loader.detect_cycle(service_name)
+        resolver.detect_cycle(service_name)
-        other_service_dict = other_loader.make_service_dict()
+        other_service_dict = process_service(resolver.working_dir, resolver.run())
         validate_extended_service_dict(
             other_service_dict,
             extended_config_path,
@@ -358,7 +355,7 @@ def validate_ulimits(ulimit_config):
                     "than 'hard' value".format(ulimit_config))
 
 
-def process_container_options(working_dir, service_dict):
+def process_service(working_dir, service_dict):
     service_dict = dict(service_dict)
 
     if 'volumes' in service_dict and service_dict.get('volume_driver') is None:

tests/integration/service_test.py

@@ -815,7 +815,13 @@ class ServiceTest(DockerClientTestCase):
             environment=['ONE=1', 'TWO=2', 'THREE=3'],
             env_file=['tests/fixtures/env/one.env', 'tests/fixtures/env/two.env'])
         env = create_and_start_container(service).environment
-        for k, v in {'ONE': '1', 'TWO': '2', 'THREE': '3', 'FOO': 'baz', 'DOO': 'dah'}.items():
+        for k, v in {
+            'ONE': '1',
+            'TWO': '2',
+            'THREE': '3',
+            'FOO': 'baz',
+            'DOO': 'dah'
+        }.items():
             self.assertEqual(env[k], v)
 
     @mock.patch.dict(os.environ)
@@ -823,9 +829,22 @@ class ServiceTest(DockerClientTestCase):
         os.environ['FILE_DEF'] = 'E1'
         os.environ['FILE_DEF_EMPTY'] = 'E2'
         os.environ['ENV_DEF'] = 'E3'
-        service = self.create_service('web', environment={'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': None, 'NO_DEF': None})
+        service = self.create_service(
+            'web',
+            environment={
+                'FILE_DEF': 'F1',
+                'FILE_DEF_EMPTY': '',
+                'ENV_DEF': None,
+                'NO_DEF': None
+            }
+        )
         env = create_and_start_container(service).environment
-        for k, v in {'FILE_DEF': 'F1', 'FILE_DEF_EMPTY': '', 'ENV_DEF': 'E3', 'NO_DEF': ''}.items():
+        for k, v in {
+            'FILE_DEF': 'F1',
+            'FILE_DEF_EMPTY': '',
+            'ENV_DEF': 'E3',
+            'NO_DEF': ''
+        }.items():
             self.assertEqual(env[k], v)
 
     def test_with_high_enough_api_version_we_get_default_network_mode(self):

tests/integration/testcases.py

@@ -7,7 +7,8 @@ from pytest import skip
 
 from .. import unittest
 from compose.cli.docker_client import docker_client
-from compose.config.config import ServiceLoader
+from compose.config.config import process_service
+from compose.config.config import resolve_environment
 from compose.const import LABEL_PROJECT
 from compose.progress_stream import stream_output
 from compose.service import Service
@@ -42,23 +43,15 @@ class DockerClientTestCase(unittest.TestCase):
         if 'command' not in kwargs:
             kwargs['command'] = ["top"]
 
-        workaround_options = {}
+        # TODO: remove this once #2299 is fixed
-        for option in ['links', 'volumes_from', 'net']:
+        kwargs['name'] = name
-            if option in kwargs:
-                workaround_options[option] = kwargs.pop(option, None)
-
-        options = ServiceLoader(
-            working_dir='.',
-            filename=None,
-            service_name=name,
-            service_dict=kwargs
-        ).make_service_dict()
-        options.update(workaround_options)
 
+        options = process_service('.', kwargs)
+        options['environment'] = resolve_environment('.', kwargs)
         labels = options.setdefault('labels', {})
         labels['com.docker.compose.test-name'] = self.id()
 
-        return Service(project='composetest', client=self.client, **options)
+        return Service(client=self.client, project='composetest', **options)
 
     def check_build(self, *args, **kwargs):
         kwargs.setdefault('rm', True)

tests/unit/config/config_test.py

@@ -18,13 +18,14 @@ from tests import unittest
 
 def make_service_dict(name, service_dict, working_dir, filename=None):
     """
-    Test helper function to construct a ServiceLoader
+    Test helper function to construct a ServiceExtendsResolver
     """
-    return config.ServiceLoader(
+    resolver = config.ServiceExtendsResolver(
         working_dir=working_dir,
         filename=filename,
         service_name=name,
-        service_dict=service_dict).make_service_dict()
+        service_dict=service_dict)
+    return config.process_service(working_dir, resolver.run())
 
 
 def service_sort(services):