Turn naming.Naming into an abstract class (#341)

* Turn naming.Naming into an abstract class

The new style client libraries have a directory/module structure of
the form:

apiName:
- __init__.py <alias to the most recent version>
apiName_version:
- __init__.py <load all the types and submodules>

Certain client libraries need to preserve a legacy interface with a
directory structure of the form

apiName:
- version

This change abstracts out the Naming class to make make the change feasible.

.coveragerc

@@ -16,4 +16,5 @@ exclude_lines =
     def __repr__
     # Abstract methods by definition are not invoked
     @abstractmethod
+    @abc.abstractmethod
 

gapic/generator/options.py

@@ -37,10 +37,12 @@ class Options:
     sample_configs: Tuple[str, ...] = dataclasses.field(default=())
     templates: Tuple[str, ...] = dataclasses.field(default=('DEFAULT',))
     lazy_import: bool = False
+    old_naming: bool = False
 
     # Class constants
     PYTHON_GAPIC_PREFIX: str = 'python-gapic-'
     OPT_FLAGS: FrozenSet[str] = frozenset((
+        'old-naming',           # TODO(dovs): Come up with a better comment
         'retry-config',         # takes a path
         'samples',              # output dir
         'lazy-import',          # requires >= 3.7
@@ -115,7 +117,8 @@ def build(cls, opt_string: str) -> 'Options':
                 for cfg_path in samplegen_utils.generate_all_sample_fpaths(s)
             ),
             templates=tuple(os.path.expanduser(i) for i in templates),
-            lazy_import=bool(opts.pop('lazy-import', False))
+            lazy_import=bool(opts.pop('lazy-import', False)),
+            old_naming=bool(opts.pop('old-naming', False)),
         )
 
         # Note: if we ever need to recursively check directories for sample

gapic/schema/metadata.py

@@ -44,7 +44,7 @@ class Address:
     package: Tuple[str, ...] = dataclasses.field(default_factory=tuple)
     parent: Tuple[str, ...] = dataclasses.field(default_factory=tuple)
     api_naming: naming.Naming = dataclasses.field(
-        default_factory=naming.Naming,
+        default_factory=naming.NewNaming,
     )
     collisions: FrozenSet[str] = dataclasses.field(default_factory=frozenset)
 

gapic/schema/naming.py

@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+import abc
 import dataclasses
 import os
 import re
@@ -22,15 +23,16 @@
 from gapic import utils
 from gapic.generator import options
 
-
-@dataclasses.dataclass(frozen=True)
-class Naming:
+# See https://github.com/python/mypy/issues/5374 for details on the mypy false
+# positive.
+@dataclasses.dataclass(frozen=True)  # type: ignore
+class Naming(abc.ABC):
     """Naming data for an API.
 
     This class contains the naming nomenclature used for this API
     within templates.
 
-    An instance of this object is made available to every template
+    An concrete child of this object is made available to every template
     (as ``api.naming``).
     """
     name: str = ''
@@ -43,11 +45,11 @@ def __post_init__(self):
         if not self.product_name:
             self.__dict__['product_name'] = self.name
 
-    @classmethod
-    def build(cls,
-              *file_descriptors: descriptor_pb2.FileDescriptorProto,
-              opts: options.Options = options.Options(),
-              ) -> 'Naming':
+    @staticmethod
+    def build(
+        *file_descriptors: descriptor_pb2.FileDescriptorProto,
+        opts: options.Options = options.Options(),
+    ) -> 'Naming':
         """Return a full Naming instance based on these file descriptors.
 
         This is pieced together from the proto package names as well as the
@@ -103,10 +105,12 @@ def build(cls,
         match = cast(Match,
                      re.search(pattern=pattern, string=root_package)).groupdict()
         match['namespace'] = match['namespace'] or ''
-        package_info = cls(
+        klass = OldNaming if opts.old_naming else NewNaming
+        package_info = klass(
             name=match['name'].capitalize(),
-            namespace=tuple([i.capitalize()
-                             for i in match['namespace'].split('.') if i]),
+            namespace=tuple(
+                i.capitalize() for i in match['namespace'].split('.') if i
+            ),
             product_name=match['name'].capitalize(),
             proto_package=root_package,
             version=match.get('version', ''),
@@ -125,24 +129,24 @@ def build(cls,
         # likely make sense to many users to use dot-separated namespaces and
         # snake case, so handle that and do the right thing.
         if opts.name:
-            package_info = dataclasses.replace(package_info, name=' '.join([
+            package_info = dataclasses.replace(package_info, name=' '.join((
                 i.capitalize() for i in opts.name.replace('_', ' ').split(' ')
-            ]))
+            )))
         if opts.namespace:
-            package_info = dataclasses.replace(package_info, namespace=tuple([
+            package_info = dataclasses.replace(package_info, namespace=tuple(
                 # The join-and-split on "." here causes us to expand out
                 # dot notation that we may have been sent; e.g. a one-tuple
                 # with ('x.y',) will become a two-tuple: ('x', 'y')
                 i.capitalize() for i in '.'.join(opts.namespace).split('.')
-            ]))
+            ))
 
         # Done; return the naming information.
         return package_info
 
     def __bool__(self):
         """Return True if any of the fields are truthy, False otherwise."""
         return any(
-            [getattr(self, i.name) for i in dataclasses.fields(self)],
+            (getattr(self, i.name) for i in dataclasses.fields(self)),
         )
 
     @property
@@ -164,19 +168,18 @@ def module_namespace(self) -> Tuple[str, ...]:
     def namespace_packages(self) -> Tuple[str, ...]:
         """Return the appropriate Python namespace packages."""
         answer: List[str] = []
-        for cursor in [i.lower() for i in self.namespace]:
+        for cursor in (i.lower() for i in self.namespace):
             answer.append(f'{answer[-1]}.{cursor}' if answer else cursor)
         return tuple(answer)
 
     @property
+    @abc.abstractmethod
     def versioned_module_name(self) -> str:
         """Return the versiond module name (e.g. ``apiname_v1``).
 
         If there is no version, this is the same as ``module_name``.
         """
-        if self.version:
-            return f'{self.module_name}_{self.version}'
-        return self.module_name
+        raise NotImplementedError
 
     @property
     def warehouse_package_name(self) -> str:
@@ -186,3 +189,23 @@ def warehouse_package_name(self) -> str:
         # proper package name.
         answer = list(self.namespace) + self.name.split(' ')
         return '-'.join(answer).lower()
+
+
+class NewNaming(Naming):
+    @property
+    def versioned_module_name(self) -> str:
+        """Return the versiond module name (e.g. ``apiname_v1``).
+
+        If there is no version, this is the same as ``module_name``.
+        """
+        return self.module_name + (f'_{self.version}' if self.version else '')
+
+
+class OldNaming(Naming):
+    @property
+    def versioned_module_name(self) -> str:
+        """Return the versiond module name (e.g. ``apiname_v1``).
+
+        If there is no version, this is the same as ``module_name``.
+        """
+        return self.module_name + (f'.{self.version}' if self.version else '')

tests/unit/generator/test_generator.py

@@ -356,7 +356,9 @@ def test_samplegen_config_to_output_files(
     # Need to have the sample template visible to the generator.
     g._env.loader = jinja2.DictLoader({'sample.py.j2': ''})
 
-    api_schema = make_api(naming=naming.Naming(name='Mollusc', version='v6'))
+    api_schema = make_api(
+        naming=naming.NewNaming(name='Mollusc', version='v6')
+    )
     actual_response = g.get_response(
         api_schema,
         opts=options.Options.build('')
@@ -445,7 +447,9 @@ def test_samplegen_id_disambiguation(mock_gmtime, mock_generate_sample, fs):
     # Need to have the sample template visible to the generator.
     g._env.loader = jinja2.DictLoader({'sample.py.j2': ''})
 
-    api_schema = make_api(naming=naming.Naming(name='Mollusc', version='v6'))
+    api_schema = make_api(
+        naming=naming.NewNaming(name='Mollusc', version='v6')
+    )
     actual_response = g.get_response(
         api_schema,
         opts=options.Options.build('')
@@ -517,7 +521,9 @@ def test_generator_duplicate_samples(fs):
 
     generator = make_generator('samples=samples.yaml')
     generator._env.loader = jinja2.DictLoader({'sample.py.j2': ''})
-    api_schema = make_api(naming=naming.Naming(name='Mollusc', version='v6'))
+    api_schema = make_api(
+        naming=naming.NewNaming(name='Mollusc', version='v6')
+    )
 
     with pytest.raises(types.DuplicateSample):
         generator.get_response(
@@ -591,7 +597,7 @@ def test_dont_generate_in_code_samples(
                     name='Mollusc')],
             ),
         ),
-        naming=naming.Naming(name='Mollusc', version='v6'),
+        naming=naming.NewNaming(name='Mollusc', version='v6'),
     )
 
     # Note that we do NOT expect a clam sample.
@@ -674,4 +680,4 @@ def make_naming(**kwargs) -> naming.Naming:
     kwargs.setdefault('namespace', ('Google', 'Cloud'))
     kwargs.setdefault('version', 'v1')
     kwargs.setdefault('product_name', 'Hatstand')
-    return naming.Naming(**kwargs)
+    return naming.NewNaming(**kwargs)

tests/unit/generator/test_options.py

@@ -25,6 +25,7 @@ def test_options_empty():
     assert len(opts.templates) == 1
     assert opts.templates[0].endswith('gapic/templates')
     assert not opts.lazy_import
+    assert not opts.old_naming
 
 
 def test_options_replace_templates():
@@ -121,3 +122,8 @@ def test_options_service_config(fs):
 def test_options_lazy_import():
     opts = options.Options.build('lazy-import')
     assert opts.lazy_import
+
+
+def test_options_old_naming():
+    opts = options.Options.build('old-naming')
+    assert opts.old_naming

tests/unit/samplegen/test_integration.py

@@ -68,7 +68,7 @@ def test_generate_sample_basic():
         }
     )
 
-    api_naming = naming.Naming(
+    api_naming = naming.NewNaming(
         name="MolluscClient", namespace=("molluscs", "v1"))
     service = wrappers.Service(
         service_pb=namedtuple('service_pb', ['name'])('MolluscService'),
@@ -207,7 +207,7 @@ def test_generate_sample_basic_unflattenable():
         }
     )
 
-    api_naming = naming.Naming(
+    api_naming = naming.NewNaming(
         name="MolluscClient", namespace=("molluscs", "v1"))
     service = wrappers.Service(
         service_pb=namedtuple('service_pb', ['name'])('MolluscService'),

tests/unit/samplegen/test_samplegen.py

@@ -83,7 +83,7 @@ def test_preprocess_sample():
     # Verify that the default response is added.
     sample = {}
     api_schema = api.API(
-        naming.Naming(
+        naming.NewNaming(
             namespace=("mollusc", "cephalopod", "teuthida")
         ),
         all_protos={},

tests/unit/schema/test_api.py

@@ -1022,4 +1022,4 @@ def make_naming(**kwargs) -> naming.Naming:
     kwargs.setdefault('namespace', ('Google', 'Cloud'))
     kwargs.setdefault('version', 'v1')
     kwargs.setdefault('product_name', 'Hatstand')
-    return naming.Naming(**kwargs)
+    return naming.NewNaming(**kwargs)

tests/unit/schema/test_metadata.py

@@ -135,23 +135,23 @@ def test_address_resolve():
 def test_address_subpackage():
     addr = metadata.Address(
         package=('foo', 'bar', 'baz', 'v1', 'spam', 'eggs'),
-        api_naming=naming.Naming(proto_package='foo.bar.baz.v1'),
+        api_naming=naming.NewNaming(proto_package='foo.bar.baz.v1'),
     )
     assert addr.subpackage == ('spam', 'eggs')
 
 
 def test_address_subpackage_no_version():
     addr = metadata.Address(
         package=('foo', 'bar', 'baz', 'spam', 'eggs'),
-        api_naming=naming.Naming(proto_package='foo.bar.baz'),
+        api_naming=naming.NewNaming(proto_package='foo.bar.baz'),
     )
     assert addr.subpackage == ('spam', 'eggs')
 
 
 def test_address_subpackage_empty():
     addr = metadata.Address(
         package=('foo', 'bar', 'baz', 'v1'),
-        api_naming=naming.Naming(proto_package='foo.bar.baz.v1'),
+        api_naming=naming.NewNaming(proto_package='foo.bar.baz.v1'),
     )
     assert addr.subpackage == ()
 
@@ -188,7 +188,7 @@ def make_doc_meta(
         leading: str = '',
         trailing: str = '',
         detached: typing.List[str] = [],
-        ) -> descriptor_pb2.SourceCodeInfo.Location:
+) -> descriptor_pb2.SourceCodeInfo.Location:
     return metadata.Metadata(
         documentation=descriptor_pb2.SourceCodeInfo.Location(
             leading_comments=leading,