Merge pull request #113 from feltech/work/88-traitVersioning

Add trait versioning

Author: feltech

.editorconfig

@@ -16,7 +16,7 @@ ij_visual_guides = 72
 [*.json]
 indent_size = 2
 
-[*.yml]
+[*.y*ml]
 indent_size = 2
 
 [*.md]

RELEASE_NOTES.md

@@ -1,6 +1,36 @@
 Release Notes
 =============
 
+v1.0.0-alpha.x
+--------------
+
+### Breaking changes
+
+- Updated the YAML schema to group traits/specifications under a version
+  number key.
+  [#80](https://github.com/OpenAssetIO/OpenAssetIO-TraitGen/issues/80)
+
+### New features
+
+- Added support for trait versioning. Suffixed generated
+  trait/specification view classes with a `_vX` and trait IDs with a `.vX`
+  (where `X` is a version number), except for the first version of a
+  trait, where the ID has no version suffix, retaining backward
+  compatibility.
+  [#80](https://github.com/OpenAssetIO/OpenAssetIO-TraitGen/issues/80)
+
+- Added an optional `deprecated` field to trait and specification YAML
+  definitions, which causes a deprecation warning/annotation to be
+  generated for all versions of that trait/specification.
+  [#80](https://github.com/OpenAssetIO/OpenAssetIO-TraitGen/issues/80)
+
+### Improvements
+
+- Updated classes without a version suffix to alias version 1, but with
+  a deprecation warning/annotation, for backward compatibility.
+  [#80](https://github.com/OpenAssetIO/OpenAssetIO-TraitGen/issues/80)
+
+
 v1.0.0-alpha.12
 --------------
 

python/openassetio_traitgen/__init__.py

@@ -176,10 +176,10 @@ def _log_package_declaration(package, logger):
         for namespace in package.traits:
             logger.info(f"{namespace.id}:")
             for trait in namespace.members:
-                logger.info("  - %s", trait.name)
+                logger.info("  - %s (v%s)", trait.name, trait.version)
     if package.specifications:
         logger.info("Specifications:")
         for namespace in package.specifications:
             logger.info(f"{namespace.id}:")
             for specification in namespace.members:
-                logger.info("  - %s", specification.id)
+                logger.info("  - %s (v%s)", specification.id, specification.version)

python/openassetio_traitgen/datamodel.py

@@ -82,6 +82,10 @@ class TraitDeclaration(NamedTuple):
     # A short name for the Trait that is only unique within its
     # namespace.
     name: str
+    # Whether this trait is deprecated.
+    deprecated: bool
+    # Version of the trait.
+    version: str
     # A user-facing description of the Trait and its purpose.
     description: str
     # User-facing hints as to the usage of this trait, in relation to
@@ -115,14 +119,16 @@ class TraitReference(NamedTuple):
     namespace: str
     # The package the trait belongs to
     package: str
+    # Version of the trait
+    version: str
     # The shortest list of elements from package, namespace and name
     # that is required to form a unique name for this trait
     # relative to the specification. These should be used
     # when building accessor method names to retrieve a
     # Trait instance from a Specification, as it handles the
     # case where a Specification may reference two identically
     # named traits in different packages or namespaces.
-    unique_name_parts: Tuple[str]
+    unique_name_parts: Tuple[str, ...]
 
 
 class SpecificationDeclaration(NamedTuple):
@@ -132,6 +138,10 @@ class SpecificationDeclaration(NamedTuple):
 
     # The unique name of Specification within its namespace.
     id: str
+    # Whether this specification is deprecated.
+    deprecated: bool
+    # Version of the specification.
+    version: str
     # A user-facing description of the Specification and its purpose.
     description: str
     # User-facing hints as to the usage of this trait, in relation to

python/openassetio_traitgen/generators/cpp.py

@@ -17,6 +17,9 @@
 A traitgen generator that outputs a C++ package based on the
 openassetio_traitgen PackageDefinition model.
 """
+import collections
+import itertools
+
 # TODO(DF): Refactor to pull out common code, then remove this
 #  suppression.
 # pylint: disable=duplicate-code
@@ -180,12 +183,27 @@ def __render_namespace(
         )
         imports = []
 
-        # Render a file per class (trait or specification).
-        for declaration in namespace.members:
+        # We group multiple versions of the same trait (or
+        # specification) together to render in the same header. Note
+        # that declarations in a namespace are already sorted
+        # appropriately by name, so we don't need to sort before
+        # applying groupby.
+        declarations_by_name = itertools.groupby(
+            namespace.members,
+            lambda declaration: declaration.name if kind == "traits" else declaration.id,
+        )
+
+        # Render a file per trait/specification, containing all
+        # versions of that trait/specification.
+        for name, declarations in declarations_by_name:
             if kind == "traits":
-                file_name = self.__render_trait(namespace, declaration, namespace_abs_path)
+                file_name = self.__render_trait(
+                    namespace, name, tuple(declarations), namespace_abs_path
+                )
             else:
-                file_name = self.__render_specification(namespace, declaration, namespace_abs_path)
+                file_name = self.__render_specification(
+                    namespace, name, tuple(declarations), namespace_abs_path
+                )
 
             imports.append(f"{namespace_name}/{file_name}")
 
@@ -207,7 +225,8 @@ def __render_namespace(
     def __render_trait(
         self,
         namespace: NamespaceDeclaration,
-        declaration: TraitDeclaration,
+        name: str,
+        declarations: tuple[TraitDeclaration, ...],
         namespace_abs_path: str,
     ) -> str:
         """
@@ -216,24 +235,25 @@ def __render_trait(
         Creates a single header file containing a single trait view
         class.
         """
-        cls_name = self.__env.filters["to_cpp_class_name"](declaration.name) + "Trait"
+        header_name = self.__env.filters["to_cpp_class_name"](name) + "Trait"
         self.__render_template(
             "trait",
-            os.path.join(namespace_abs_path, f"{cls_name}.hpp"),
+            os.path.join(namespace_abs_path, f"{header_name}.hpp"),
             {
                 "package": self.__package,
                 "namespace": namespace,
-                "trait": declaration,
+                "versions": declarations,
                 "openassetio_abi_version": OPENASSETIO_ABI_VERSION,
                 "traitgen_abi_version": TRAITGEN_ABI_VERSION,
             },
         )
-        return f"{cls_name}.hpp"
+        return f"{header_name}.hpp"
 
     def __render_specification(
         self,
         namespace: NamespaceDeclaration,
-        declaration: SpecificationDeclaration,
+        name: str,
+        declarations: tuple[SpecificationDeclaration, ...],
         namespace_abs_path: str,
     ) -> str:
         """
@@ -242,19 +262,38 @@ def __render_specification(
         Creates a single header file containing a single specification
         class.
         """
-        cls_name = self.__env.filters["to_cpp_class_name"](declaration.id) + "Specification"
+
+        # Properties required to interpolate when constructing #include
+        # directives.
+        TraitHeaderPathTokens = collections.namedtuple(
+            "TraitHeaderPathTokens", ("package", "namespace", "name")
+        )
+
+        # All versions of a given trait live in a single header.
+        # Extract fields required to #include the trait headers
+        # referenced by all versions of this specification, de-duped.
+        all_trait_header_path_tokens = sorted(
+            {
+                TraitHeaderPathTokens(trait_decl.package, trait_decl.namespace, trait_decl.name)
+                for spec_decl in declarations
+                for trait_decl in spec_decl.trait_set
+            }
+        )
+
+        header_name = self.__env.filters["to_cpp_class_name"](name) + "Specification"
         self.__render_template(
             "specification",
-            os.path.join(namespace_abs_path, f"{cls_name}.hpp"),
+            os.path.join(namespace_abs_path, f"{header_name}.hpp"),
             {
                 "package": self.__package,
                 "namespace": namespace,
-                "specification": declaration,
+                "versions": declarations,
+                "all_trait_header_path_tokens": all_trait_header_path_tokens,
                 "openassetio_abi_version": OPENASSETIO_ABI_VERSION,
                 "traitgen_abi_version": TRAITGEN_ABI_VERSION,
             },
         )
-        return f"{cls_name}.hpp"
+        return f"{header_name}.hpp"
 
     def __render_package_template(
         self, package_abs_path: str, name: str, docstring: str, imports: List[str]

python/openassetio_traitgen/generators/python.py

@@ -181,7 +181,7 @@ def to_py_module_name(string: str):
         no_hypens = string.replace("-", "_")
         module_name = re.sub(r"[^a-zA-Z0-9_]", "_", no_hypens)
         if module_name != no_hypens:
-            logger.warning(f"Conforming '{string}' to '{module_name}' for module name")
+            _conform_warning(string, module_name, "module name")
         return module_name
 
     def to_py_class_name(string: str):
@@ -190,7 +190,7 @@ def to_py_class_name(string: str):
         """
         class_name = helpers.to_upper_camel_alnum(string)
         if class_name != string:
-            logger.warning(f"Conforming '{string}' to '{class_name}' for class name")
+            _conform_warning(string, class_name, "class name")
         validate_identifier(class_name, string)
         return class_name
 
@@ -204,9 +204,7 @@ def to_py_trait_accessor_name(name_parts: List[str]):
         accessor_name = helpers.to_lower_camel_alnum(unique_name)
         # We expect the first letter to change to lowercase
         if accessor_name != f"{unique_name[0].lower()}{unique_name[1:]}":
-            logger.warning(
-                f"Conforming '{unique_name}' to '{accessor_name}' for trait getter name"
-            )
+            _conform_warning(unique_name, accessor_name, "trait getter name")
         validate_identifier(accessor_name, unique_name)
         return accessor_name
 
@@ -218,9 +216,7 @@ def to_py_var_accessor_name(string: str):
         """
         accessor_name = helpers.to_upper_camel_alnum(string)
         if accessor_name != f"{string[0].upper()}{string[1:]}":
-            logger.warning(
-                f"Conforming '{string}' to '{accessor_name}' for property accessor name"
-            )
+            _conform_warning(string, accessor_name, "property accessor name")
         validate_identifier(accessor_name, string)
         return accessor_name
 
@@ -231,7 +227,7 @@ def to_py_var_name(string: str):
         """
         var_name = helpers.to_lower_camel_alnum(string)
         if var_name != string:
-            logger.warning(f"Conforming '{string}' to '{var_name}' for variable name")
+            _conform_warning(string, var_name, "variable name")
         validate_identifier(var_name, string)
         return var_name
 
@@ -251,6 +247,19 @@ def to_py_type(declaration_type):
             raise TypeError("Dictionary types are not yet supported as trait properties")
         return type_map[declaration_type]
 
+    def _conform_warning(original: str, conformed: str, context: str):
+        """
+        Log a warning that an input name has been modified to conform
+        to a valid identifier, if the warning has not already been
+        logged.
+        """
+        warning = f"Conforming '{original}' to '{conformed}' for {context}"
+        if warning in environment.globals["conform_warnings"]:
+            return
+        environment.globals["conform_warnings"].append(warning)
+        logger.warning(warning)
+
+    environment.globals["conform_warnings"] = []
     environment.filters["to_upper_camel_alnum"] = helpers.to_upper_camel_alnum
     environment.filters["to_py_module_name"] = to_py_module_name
     environment.filters["to_py_class_name"] = to_py_class_name

python/openassetio_traitgen/parser.py

@@ -88,11 +88,14 @@ def _unpack_specifications(model: dict, package_id: str) -> List[datamodel.Names
         specifications = [
             datamodel.SpecificationDeclaration(
                 id=name,
+                deprecated=props.get("deprecated", False),
+                version=version_num,
                 description=definition.get("description", "").strip(),
                 trait_set=_unpack_trait_set(definition["traitSet"], package_id),
                 usage=definition.get("usage", []),
             )
-            for name, definition in data["members"].items()
+            for name, props in data["members"].items()
+            for version_num, definition in props["versions"].items()
         ]
         specifications.sort(key=_byId)
 
@@ -134,8 +137,9 @@ def _unpack_trait_set(trait_set: List[dict], package_id: str) -> List[datamodel.
         package = trait.get("package", package_id)
         namespace = trait["namespace"]
         name = trait["name"]
+        version = trait["version"]
 
-        identifier = _build_trait_id(package, namespace, name)
+        identifier = _build_trait_id(package, namespace, name, version)
 
         # Check to see which of the possible combinations of reference
         # parts is unique for this trait.
@@ -153,6 +157,7 @@ def _unpack_trait_set(trait_set: List[dict], package_id: str) -> List[datamodel.
                 name=name,
                 namespace=namespace,
                 package=package,
+                version=version,
                 unique_name_parts=unique_name_parts,
             )
         )
@@ -162,7 +167,15 @@ def _unpack_trait_set(trait_set: List[dict], package_id: str) -> List[datamodel.
     return references
 
 
-def _build_trait_id(package: str, namespace: str, name: str) -> str:
+def _build_trait_id(package: str, namespace: str, name: str, version: str) -> str:
+    """
+    Builds a trait ID from the supplied components.
+
+    The first version "1" omits the version suffix to maintain backward
+    compatibility with existing traits.
+    """
+    if version != "1":
+        return f"{package}:{namespace}.{name}.v{version}"
     return f"{package}:{namespace}.{name}"
 
 
@@ -180,13 +193,16 @@ def _unpack_traits(
     for namespace, data in model.items():
         traits = [
             datamodel.TraitDeclaration(
-                id=_build_trait_id(package_id, namespace, name),
+                id=_build_trait_id(package_id, namespace, name, version_num),
                 name=name,
+                deprecated=props.get("deprecated", False),
+                version=version_num,
                 description=definition.get("description", "").strip(),
                 properties=_unpack_properties(definition.get("properties", {})),
                 usage=definition.get("usage", []),
             )
-            for name, definition in data["members"].items()
+            for name, props in data["members"].items()
+            for version_num, definition in props["versions"].items()
         ]
         traits.sort(key=_byName)