Merge ~cjwatson/launchpad:schema-circular-imports-decentralize-remaining into launchpad:master

[Colin Watson]

Split up remaining circular import workarounds

I removed an unnecessary bit of circular import avoidance in `lp.services.comments.interfaces.conversation`.

[Andrey Fedoseev (andrey-fedoseev)]

review: Approve

diff --git a/lib/lp/_schema_circular_imports.py b/lib/lp/_schema_circular_imports.py
deleted file mode 100644
index 9231cad..0000000
--- a/lib/lp/_schema_circular_imports.py
+++ /dev/null
@@ -1,54 +0,0 @@
-# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Update the interface schema values due to circular imports.
-
-There are situations where there would normally be circular imports to define
-the necessary schema values in some interface fields.  To avoid this the
-schema is initially set to `Interface`, but this needs to be updated once the
-types are defined.
-"""
-
-__all__ = []
-
-from lp.bugs.interfaces.bugtask import IBugTask
-from lp.buildmaster.interfaces.builder import IBuilder
-from lp.buildmaster.interfaces.buildfarmjob import IBuildFarmJob
-from lp.buildmaster.interfaces.buildqueue import IBuildQueue
-from lp.code.interfaces.gitrepository import IGitRepository
-from lp.registry.interfaces.person import IPerson
-from lp.services.auth.interfaces import IAccessToken
-from lp.services.comments.interfaces.conversation import IComment
-from lp.services.messages.interfaces.message import (
-    IIndexedMessage,
-    IMessage,
-    IUserToUserEmail,
-)
-from lp.services.messages.interfaces.messagerevision import IMessageRevision
-from lp.services.webservice.apihelpers import (
-    patch_collection_property,
-    patch_reference_property,
-)
-
-# IBuilder
-patch_reference_property(IBuilder, "current_build", IBuildFarmJob)
-
-# IBuildFarmJob
-patch_reference_property(IBuildFarmJob, "buildqueue_record", IBuildQueue)
-
-# IComment
-patch_reference_property(IComment, "comment_author", IPerson)
-
-# IIndexedMessage
-patch_reference_property(IIndexedMessage, "inside", IBugTask)
-
-# IMessage
-patch_reference_property(IMessage, "owner", IPerson)
-patch_collection_property(IMessage, "revisions", IMessageRevision)
-
-# IUserToUserEmail
-patch_reference_property(IUserToUserEmail, "sender", IPerson)
-patch_reference_property(IUserToUserEmail, "recipient", IPerson)
-
-# IAccessToken
-patch_reference_property(IAccessToken, "git_repository", IGitRepository)
diff --git a/lib/lp/buildmaster/interfaces/builder.py b/lib/lp/buildmaster/interfaces/builder.py
index 09e548d..75cfcfd 100644
--- a/lib/lp/buildmaster/interfaces/builder.py
+++ b/lib/lp/buildmaster/interfaces/builder.py
@@ -232,7 +232,9 @@ class IBuilderView(IHasBuildRecords, IHasOwner):
             title=_("Current build"),
             required=False,
             readonly=True,
-            schema=Interface,  # Really IBuildFarmJob.
+            # Really IBuildFarmJob, patched in
+            # lp.buildmaster.interfaces.webservice.
+            schema=Interface,
             description=_("The job currently running on this builder."),
         ),
         as_of="devel",
diff --git a/lib/lp/buildmaster/interfaces/buildfarmjob.py b/lib/lp/buildmaster/interfaces/buildfarmjob.py
index f4200bd..f08ae96 100644
--- a/lib/lp/buildmaster/interfaces/buildfarmjob.py
+++ b/lib/lp/buildmaster/interfaces/buildfarmjob.py
@@ -184,8 +184,7 @@ class IBuildFarmJobView(Interface):
     )
 
     buildqueue_record = Reference(
-        # Really IBuildQueue, set in _schema_circular_imports to avoid
-        # circular import.
+        # Really IBuildQueue, patched in lp.buildmaster.interfaces.webservice.
         schema=Interface,
         required=True,
         title=_("Corresponding BuildQueue record"),
diff --git a/lib/lp/buildmaster/interfaces/webservice.py b/lib/lp/buildmaster/interfaces/webservice.py
index 87d2c10..bd5d08c 100644
--- a/lib/lp/buildmaster/interfaces/webservice.py
+++ b/lib/lp/buildmaster/interfaces/webservice.py
@@ -25,4 +25,12 @@ from lp.buildmaster.interfaces.buildfarmjob import (
     CannotBeRetried,
     IBuildFarmJob,
 )
+from lp.buildmaster.interfaces.buildqueue import IBuildQueue
 from lp.buildmaster.interfaces.processor import IProcessor, IProcessorSet
+from lp.services.webservice.apihelpers import patch_reference_property
+
+# IBuilder
+patch_reference_property(IBuilder, "current_build", IBuildFarmJob)
+
+# IBuildFarmJob
+patch_reference_property(IBuildFarmJob, "buildqueue_record", IBuildQueue)
diff --git a/lib/lp/configure.zcml b/lib/lp/configure.zcml
index a646f98..b8467e3 100644
--- a/lib/lp/configure.zcml
+++ b/lib/lp/configure.zcml
@@ -41,7 +41,6 @@
 
     <include file="permissions.zcml" />
 
-    <webservice:register module="lp.patchwebservice" />
     <authorizations module="lp.security" />
 
     <!-- The default Zope 3 configuration of the SimpleComponentTraverser is
diff --git a/lib/lp/patchwebservice.py b/lib/lp/patchwebservice.py
deleted file mode 100644
index 20c84fb..0000000
--- a/lib/lp/patchwebservice.py
+++ /dev/null
@@ -1,16 +0,0 @@
-# Copyright 2010-2011 Canonical Ltd.  This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""All the interfaces that are exposed through the webservice.
-
-There is a declaration in ZCML somewhere that looks like:
-  <webservice:register module="lp.patchwebservice" />
-
-which tells `lazr.restful` that it should look for webservice exports here.
-"""
-
-# 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
-
-_schema_circular_imports
diff --git a/lib/lp/services/auth/interfaces.py b/lib/lp/services/auth/interfaces.py
index 471d458..07e7c77 100644
--- a/lib/lp/services/auth/interfaces.py
+++ b/lib/lp/services/auth/interfaces.py
@@ -69,7 +69,7 @@ class IAccessToken(Interface):
     git_repository = Reference(
         title=_("Git repository"),
         description=_("The Git repository for which the token was issued."),
-        # Really IGitRepository, patched in _schema_circular_imports.py.
+        # Really IGitRepository, patched in lp.services.auth.webservice.
         schema=Interface,
         required=True,
         readonly=True,
diff --git a/lib/lp/services/auth/webservice.py b/lib/lp/services/auth/webservice.py
index ceb378a..99103cc 100644
--- a/lib/lp/services/auth/webservice.py
+++ b/lib/lp/services/auth/webservice.py
@@ -8,4 +8,9 @@ __all__ = [
     "IAccessTokenTarget",
 ]
 
+from lp.code.interfaces.gitrepository import IGitRepository
 from lp.services.auth.interfaces import IAccessToken, IAccessTokenTarget
+from lp.services.webservice.apihelpers import patch_reference_property
+
+# IAccessToken
+patch_reference_property(IAccessToken, "git_repository", IGitRepository)
diff --git a/lib/lp/services/comments/interfaces/conversation.py b/lib/lp/services/comments/interfaces/conversation.py
index 5bccf9a..24712fa 100644
--- a/lib/lp/services/comments/interfaces/conversation.py
+++ b/lib/lp/services/comments/interfaces/conversation.py
@@ -14,6 +14,7 @@ from zope.interface import Interface
 from zope.schema import Bool, Datetime, Int, Text, TextLine
 
 from lp import _
+from lp.registry.interfaces.person import IPerson
 
 
 class IComment(Interface):
@@ -59,10 +60,7 @@ class IComment(Interface):
     )
 
     comment_author = Reference(
-        # Really IPerson.
-        Interface,
-        title=_("The author of the comment."),
-        readonly=True,
+        IPerson, title=_("The author of the comment."), readonly=True
     )
 
     comment_date = Datetime(title=_("Comment date."), readonly=True)
diff --git a/lib/lp/services/identity/interfaces/webservice.py b/lib/lp/services/identity/interfaces/webservice.py
index 966bf65..fc32bc2 100644
--- a/lib/lp/services/identity/interfaces/webservice.py
+++ b/lib/lp/services/identity/interfaces/webservice.py
@@ -4,7 +4,7 @@
 """All the interfaces that are exposed through the webservice.
 
 There is a declaration in ZCML somewhere that looks like:
-  <webservice:register module="lp.patchwebservice" />
+  <webservice:register module="lp.services.identity.interfaces.webservice" />
 
 which tells `lazr.restful` that it should look for webservice exports here.
 """
diff --git a/lib/lp/services/messages/interfaces/message.py b/lib/lp/services/messages/interfaces/message.py
index fc83ad3..354a84e 100644
--- a/lib/lp/services/messages/interfaces/message.py
+++ b/lib/lp/services/messages/interfaces/message.py
@@ -77,7 +77,12 @@ class IMessageCommon(Interface):
     )
     owner = exported(
         Reference(
-            title=_("Person"), schema=Interface, required=False, readonly=True
+            title=_("Person"),
+            # Really IPerson, patched in
+            # lp.services.messages.interfaces.webservice.
+            schema=Interface,
+            required=False,
+            readonly=True,
         )
     )
 
@@ -87,7 +92,8 @@ class IMessageCommon(Interface):
             description=_(
                 "Revision history of this message, sorted in ascending order."
             ),
-            # Really IMessageRevision, patched in _schema_circular_imports.
+            # Really IMessageRevision, patched in
+            # lp.services.messages.interfaces.webservice.
             value_type=Reference(schema=Interface),
             required=False,
             readonly=True,
@@ -241,10 +247,10 @@ class IIndexedMessage(Interface):
 
     inside = Reference(
         title=_("Inside"),
+        # Really IBugTask, patched in
+        # lp.services.messages.interfaces.webservice.
         schema=Interface,
-        description=_(
-            "The bug task which is " "the context for this message."
-        ),
+        description=_("The bug task which is the context for this message."),
         required=True,
         readonly=True,
     )
@@ -294,6 +300,8 @@ class IUserToUserEmail(Interface):
     """User to user direct email communications."""
 
     sender = Object(
+        # Really IPerson, patched in
+        # lp.services.messages.interfaces.webservice.
         schema=Interface,
         title=_("The message sender"),
         required=True,
@@ -301,6 +309,8 @@ class IUserToUserEmail(Interface):
     )
 
     recipient = Object(
+        # Really IPerson, patched in
+        # lp.services.messages.interfaces.webservice.
         schema=Interface,
         title=_("The message recipient"),
         required=True,
diff --git a/lib/lp/services/messages/interfaces/webservice.py b/lib/lp/services/messages/interfaces/webservice.py
index 2eb1a38..420658e 100644
--- a/lib/lp/services/messages/interfaces/webservice.py
+++ b/lib/lp/services/messages/interfaces/webservice.py
@@ -4,7 +4,7 @@
 """All the interfaces that are exposed through the webservice.
 
 There is a declaration in ZCML somewhere that looks like:
-  <webservice:register module="lp.patchwebservice" />
+  <webservice:register module="lp.services.messages.interfaces.webservice" />
 
 which tells `lazr.restful` that it should look for webservice exports here.
 """
@@ -14,8 +14,26 @@ __all__ = [
     "IMessageRevision",
 ]
 
-from lp import _schema_circular_imports
-from lp.services.messages.interfaces.message import IMessage
+from lp.bugs.interfaces.bugtask import IBugTask
+from lp.registry.interfaces.person import IPerson
+from lp.services.messages.interfaces.message import (
+    IIndexedMessage,
+    IMessage,
+    IUserToUserEmail,
+)
 from lp.services.messages.interfaces.messagerevision import IMessageRevision
+from lp.services.webservice.apihelpers import (
+    patch_collection_property,
+    patch_reference_property,
+)
 
-_schema_circular_imports
+# IIndexedMessage
+patch_reference_property(IIndexedMessage, "inside", IBugTask)
+
+# IMessage
+patch_reference_property(IMessage, "owner", IPerson)
+patch_collection_property(IMessage, "revisions", IMessageRevision)
+
+# IUserToUserEmail
+patch_reference_property(IUserToUserEmail, "sender", IPerson)
+patch_reference_property(IUserToUserEmail, "recipient", IPerson)
diff --git a/lib/lp/services/temporaryblobstorage/webservice.py b/lib/lp/services/temporaryblobstorage/webservice.py
index 5218c46..7f03a9f 100644
--- a/lib/lp/services/temporaryblobstorage/webservice.py
+++ b/lib/lp/services/temporaryblobstorage/webservice.py
@@ -4,7 +4,7 @@
 """All the interfaces that are exposed through the webservice.
 
 There is a declaration in ZCML somewhere that looks like:
-  <webservice:register module="lp.patchwebservice" />
+  <webservice:register module="lp.services.temporaryblobstorage.webservice" />
 
 which tells `lazr.restful` that it should look for webservice exports here.
 """
diff --git a/lib/lp/services/webservice/webservice.py b/lib/lp/services/webservice/webservice.py
index a798f99..74a18a0 100644
--- a/lib/lp/services/webservice/webservice.py
+++ b/lib/lp/services/webservice/webservice.py
@@ -4,7 +4,7 @@
 """All the interfaces that are exposed through the webservice.
 
 There is a declaration in ZCML somewhere that looks like:
-  <webservice:register module="lp.patchwebservice" />
+  <webservice:register module="lp.services.webservice.webservice" />
 
 which tells `lazr.restful` that it should look for webservice exports here.
 """
diff --git a/lib/lp/services/worlddata/interfaces/webservice.py b/lib/lp/services/worlddata/interfaces/webservice.py
index 1ff5d14..036d5c9 100644
--- a/lib/lp/services/worlddata/interfaces/webservice.py
+++ b/lib/lp/services/worlddata/interfaces/webservice.py
@@ -16,10 +16,5 @@ __all__ = [
     "ILanguageSet",
 ]
 
-# 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.services.worlddata.interfaces.country import ICountry, ICountrySet
 from lp.services.worlddata.interfaces.language import ILanguage, ILanguageSet
-
-_schema_circular_imports