Merge ~twom/launchpad:add-restricted-files-to-spph-api into launchpad:master

[Tom Wardill]

Add has_restricted_files to SPPH API.

[Colin Watson (cjwatson)]

Can this be exported as a method instead? The implementation isn't quite so trivial that I feel entirely comfortable exporting it as an attribute (which would mean that it'd be called any time somebody asks for one of these objects over the webservice).

In fact, the current interface claims that it's an attribute but the implementation is a non-property method, which probably won't quite work right. Consider renaming this to `hasRestrictedFiles` and fixing the interface to be a method with `@export_read_operation`.

Perhaps this could also do with a webservice test in `lp.soyuz.browser.tests.test_publishing_webservice`.

review: Needs Information

[Colin Watson (cjwatson)]

review: Approve

diff --git a/lib/lp/soyuz/browser/tests/test_publishing_webservice.py b/lib/lp/soyuz/browser/tests/test_publishing_webservice.py
index ee74d72..5a43e2f 100644
--- a/lib/lp/soyuz/browser/tests/test_publishing_webservice.py
+++ b/lib/lp/soyuz/browser/tests/test_publishing_webservice.py
@@ -7,6 +7,8 @@ from __future__ import absolute_import, print_function, unicode_literals
 
 from functools import partial
 
+from zope.security.proxy import removeSecurityProxy
+
 from lp.services.librarian.browser import ProxiedLibraryFileAlias
 from lp.services.webapp.interfaces import OAuthPermission
 from lp.soyuz.adapters.proxiedsourcefiles import ProxiedSourceLibraryFileAlias
@@ -82,6 +84,26 @@ class SourcePackagePublishingHistoryWebserviceTests(TestCaseWithFactory):
                 } for sprf in spph.sourcepackagerelease.files]
         self.assertContentEqual(expected_info, info)
 
+    def test_hasRestrictedFiles(self):
+        person = self.factory.makePerson()
+        webservice = webservice_for_person(
+            person, permission=OAuthPermission.READ_PUBLIC)
+        spph, url = self.make_spph_for(person)
+
+        response = webservice.named_get(
+            url, 'hasRestrictedFiles', api_version='devel')
+        self.assertEqual(200, response.status)
+        self.assertFalse(response.jsonBody())
+
+        with person_logged_in(person):
+            sprf = spph.sourcepackagerelease.files[0]
+            removeSecurityProxy(sprf.libraryfile).restricted = True
+
+        response = webservice.named_get(
+            url, 'hasRestrictedFiles', api_version='devel')
+        self.assertEqual(200, response.status)
+        self.assertTrue(response.jsonBody())
+
 
 class BinaryPackagePublishingHistoryWebserviceTests(TestCaseWithFactory):
 
diff --git a/lib/lp/soyuz/interfaces/publishing.py b/lib/lp/soyuz/interfaces/publishing.py
index fe1c424..eae5343 100644
--- a/lib/lp/soyuz/interfaces/publishing.py
+++ b/lib/lp/soyuz/interfaces/publishing.py
@@ -428,6 +428,11 @@ class ISourcePackagePublishingHistoryPublic(IPublishingView):
             `IBinaryPackagePublishingHistory`.
         """
 
+    @export_read_operation()
+    @operation_for_version("devel")
+    def hasRestrictedFiles():
+        """Return whether or not a given source files has restricted files."""
+
     # Really IBuild (fixed in _schema_circular_imports.py)
     @operation_returns_collection_of(Interface)
     @export_read_operation()
diff --git a/lib/lp/soyuz/model/publishing.py b/lib/lp/soyuz/model/publishing.py
index 2c84740..3890bb0 100644
--- a/lib/lp/soyuz/model/publishing.py
+++ b/lib/lp/soyuz/model/publishing.py
@@ -619,6 +619,19 @@ class SourcePackagePublishingHistory(SQLBase, ArchivePublisherBase):
         getUtility(IPublishingSet).requestDeletion(
             [self], removed_by, removal_comment)
 
+    def hasRestrictedFiles(self):
+        """See ISourcePackagePublishingHistory."""
+        for source_file in self.sourcepackagerelease.files:
+            if source_file.libraryfile.restricted:
+                return True
+
+        for binary in self.getBuiltBinaries():
+            for binary_file in binary.binarypackagerelease.files:
+                if binary_file.libraryfile.restricted:
+                    return True
+
+        return False
+
 
 @implementer(IBinaryPackagePublishingHistory)
 class BinaryPackagePublishingHistory(SQLBase, ArchivePublisherBase):
diff --git a/lib/lp/soyuz/scripts/packagecopier.py b/lib/lp/soyuz/scripts/packagecopier.py
index 2e4a767..b8293e3 100644
--- a/lib/lp/soyuz/scripts/packagecopier.py
+++ b/lib/lp/soyuz/scripts/packagecopier.py
@@ -111,21 +111,6 @@ def update_files_privacy(pub_record):
     return changed_files
 
 
-# XXX cprov 2009-07-01: should be part of `ISourcePackagePublishingHistory`.
-def has_restricted_files(source):
-    """Whether or not a given source files has restricted files."""
-    for source_file in source.sourcepackagerelease.files:
-        if source_file.libraryfile.restricted:
-            return True
-
-    for binary in source.getBuiltBinaries():
-        for binary_file in binary.binarypackagerelease.files:
-            if binary_file.libraryfile.restricted:
-                return True
-
-    return False
-
-
 @delegate_to(ISourcePackagePublishingHistory)
 class CheckedCopy:
     """Representation of a copy that was checked and approved.
@@ -508,7 +493,7 @@ class CopyChecker:
                     (ancestry.displayname, ancestry.distroseries.name))
 
         requires_unembargo = (
-            not self.archive.private and has_restricted_files(source))
+            not self.archive.private and source.hasRestrictedFiles())
 
         if requires_unembargo and not self.unembargo:
             raise CannotCopy(
@@ -663,7 +648,7 @@ def do_copy(sources, archive, series, pocket, include_binaries=False,
                 announce_from_person=announce_from_person,
                 previous_version=old_version, logger=logger)
             mailer.sendAll()
-        if not archive.private and has_restricted_files(source):
+        if not archive.private and source.hasRestrictedFiles():
             # Fix copies by unrestricting files with privacy mismatch.
             # We must do this *after* calling mailer.sendAll (which only
             # actually sends mail on commit), because otherwise the new