launchpad-reviewers team mailing list archive

[Colin Watson <mp+425882@xxxxxxxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:schema-circular-imports-decentralize-blueprints into launchpad:master.

Commit message:
Move circular import workarounds to lp.blueprints.interfaces.webservice

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)
Related bugs:
  Bug #673083 in Launchpad itself: "Break _schema_circular_imports into per-package import fixes"
  https://bugs.launchpad.net/launchpad/+bug/673083

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/425882

In some ways this is still a workaround for not being able to lazily reference interfaces in `Reference` schemas and similar, but I think it's still an improvement over the centralized blob of fixes in `lp._schema_circular_imports`.  (Of course we'd need to do the same thing for all packages.)
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:schema-circular-imports-decentralize-blueprints into launchpad:master.

diff --git a/lib/lp/_schema_circular_imports.py b/lib/lp/_schema_circular_imports.py
index 804c5e5..3b692bd 100644
--- a/lib/lp/_schema_circular_imports.py
+++ b/lib/lp/_schema_circular_imports.py
@@ -16,10 +16,6 @@ from lazr.restful.fields import Reference
 
 from lp.blueprints.interfaces.specification import ISpecification
 from lp.blueprints.interfaces.specificationbranch import ISpecificationBranch
-from lp.blueprints.interfaces.specificationtarget import (
-    IHasSpecifications,
-    ISpecificationTarget,
-    )
 from lp.bugs.interfaces.bug import (
     IBug,
     IFrontPageBugAddForm,
@@ -640,22 +636,5 @@ patch_collection_return_type(
 # IProductSeries
 patch_reference_property(IProductSeries, 'product', IProduct)
 
-# ISpecification
-patch_plain_parameter_type(ISpecification, 'linkBug', 'bug', IBug)
-patch_plain_parameter_type(ISpecification, 'unlinkBug', 'bug', IBug)
-patch_collection_property(ISpecification, 'dependencies', ISpecification)
-patch_collection_property(
-    ISpecification, 'linked_branches', ISpecificationBranch)
-
-# ISpecificationTarget
-patch_entry_return_type(
-    ISpecificationTarget, 'getSpecification', ISpecification)
-
-# IHasSpecifications
-patch_collection_property(
-    IHasSpecifications, 'visible_specifications', ISpecification)
-patch_collection_property(
-    IHasSpecifications, 'api_valid_specifications', ISpecification)
-
 # IAccessToken
 patch_reference_property(IAccessToken, 'git_repository', IGitRepository)
diff --git a/lib/lp/blueprints/interfaces/specification.py b/lib/lp/blueprints/interfaces/specification.py
index 5dbafe5..0f56e6e 100644
--- a/lib/lp/blueprints/interfaces/specification.py
+++ b/lib/lp/blueprints/interfaces/specification.py
@@ -526,7 +526,9 @@ class ISpecificationView(IHasOwner, IHasLinkedBranches):
     dependencies = exported(
         CollectionField(
             title=_("Specs on which this one depends."),
-            value_type=Reference(schema=Interface),  # ISpecification, really.
+            # Really ISpecification, patched in
+            # lp.blueprints.interfaces.webservice.
+            value_type=Reference(schema=Interface),
             readonly=True,
         ),
         as_of="devel",
@@ -537,7 +539,9 @@ class ISpecificationView(IHasOwner, IHasLinkedBranches):
                 "Branches associated with this spec, usually "
                 "branches on which this spec is being implemented."
             ),
-            value_type=Reference(schema=Interface),  # ISpecificationBranch
+            # Really ISpecificationBranch, patched in
+            # lp.blueprints.interfaces.webservice.
+            value_type=Reference(schema=Interface),
             readonly=True,
         ),
         as_of="devel",
@@ -859,7 +863,10 @@ class ISpecification(
         """
 
     @call_with(user=REQUEST_USER)
-    @operation_parameters(bug=Reference(schema=Interface))  # Really IBug
+    @operation_parameters(
+        # Really IBug, patched in lp.blueprints.interfaces.webservice.
+        bug=Reference(schema=Interface)
+    )
     @export_write_operation()
     @operation_for_version("devel")
     def linkBug(bug, user=None, check_permissions=True):
@@ -869,7 +876,10 @@ class ISpecification(
         """
 
     @call_with(user=REQUEST_USER)
-    @operation_parameters(bug=Reference(schema=Interface))  # Really IBug
+    @operation_parameters(
+        # Really IBug, patched in lp.blueprints.interfaces.webservice.
+        bug=Reference(schema=Interface)
+    )
     @export_write_operation()
     @operation_for_version("devel")
     def unlinkBug(bug, user=None, check_permissions=True):
diff --git a/lib/lp/blueprints/interfaces/specificationtarget.py b/lib/lp/blueprints/interfaces/specificationtarget.py
index 1eba6ae..2f4b472 100644
--- a/lib/lp/blueprints/interfaces/specificationtarget.py
+++ b/lib/lp/blueprints/interfaces/specificationtarget.py
@@ -36,9 +36,9 @@ class IHasSpecifications(Interface):
         doNotSnapshot(
             CollectionField(
                 title=_("All specifications"),
-                value_type=Reference(
-                    schema=Interface
-                ),  # ISpecification, really.
+                # Really ISpecification, patched in
+                # lp.blueprints.interfaces.webservice.
+                value_type=Reference(schema=Interface),
                 readonly=True,
                 description=_(
                     "A list of all specifications, regardless of status or "
@@ -54,9 +54,9 @@ class IHasSpecifications(Interface):
         doNotSnapshot(
             CollectionField(
                 title=_("Valid specifications"),
-                value_type=Reference(
-                    schema=Interface
-                ),  # ISpecification, really.
+                # Really ISpecification, patched in
+                # lp.blueprints.interfaces.webservice.
+                value_type=Reference(schema=Interface),
                 readonly=True,
                 description=_(
                     "All specifications that are not obsolete. When called "
@@ -112,7 +112,8 @@ class ISpecificationTarget(IHasSpecifications):
     @operation_parameters(
         name=TextLine(title=_("The name of the specification"))
     )
-    @operation_returns_entry(Interface)  # really ISpecification
+    # Really ISpecification, patched in lp.blueprints.interfaces.webservice.
+    @operation_returns_entry(Interface)
     @export_read_operation()
     @operation_for_version("devel")
     def getSpecification(name):
diff --git a/lib/lp/blueprints/interfaces/webservice.py b/lib/lp/blueprints/interfaces/webservice.py
index e807ca5..626bfce 100644
--- a/lib/lp/blueprints/interfaces/webservice.py
+++ b/lib/lp/blueprints/interfaces/webservice.py
@@ -18,9 +18,6 @@ __all__ = [
     "ISpecificationTarget",
 ]
 
-# XXX: JonathanLange 2010-11-09 bug=673083: Legacy work-around for circular
-# import bugs.  Break this up into a per-package thing.
-from lp import _schema_circular_imports
 from lp.blueprints.interfaces.specification import (
     GoalProposeError,
     ISpecification,
@@ -30,6 +27,34 @@ from lp.blueprints.interfaces.specificationbranch import ISpecificationBranch
 from lp.blueprints.interfaces.specificationsubscription import (
     ISpecificationSubscription,
 )
-from lp.blueprints.interfaces.specificationtarget import ISpecificationTarget
+from lp.blueprints.interfaces.specificationtarget import (
+    IHasSpecifications,
+    ISpecificationTarget,
+)
+from lp.bugs.interfaces.bug import IBug
+from lp.services.webservice.apihelpers import (
+    patch_collection_property,
+    patch_entry_return_type,
+    patch_plain_parameter_type,
+)
+
+# IHasSpecifications
+patch_collection_property(
+    IHasSpecifications, "visible_specifications", ISpecification
+)
+patch_collection_property(
+    IHasSpecifications, "api_valid_specifications", ISpecification
+)
+
+# ISpecification
+patch_plain_parameter_type(ISpecification, "linkBug", "bug", IBug)
+patch_plain_parameter_type(ISpecification, "unlinkBug", "bug", IBug)
+patch_collection_property(ISpecification, "dependencies", ISpecification)
+patch_collection_property(
+    ISpecification, "linked_branches", ISpecificationBranch
+)
 
-_schema_circular_imports
+# ISpecificationTarget
+patch_entry_return_type(
+    ISpecificationTarget, "getSpecification", ISpecification
+)