launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #24981
[Merge] ~cjwatson/launchpad:stormify-temporaryblobstorage into launchpad:master
Colin Watson has proposed merging ~cjwatson/launchpad:stormify-temporaryblobstorage into launchpad:master.
Commit message:
Convert TemporaryBlobStorage to Storm
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/387081
I applied the usual __future__ imports to some tests in the process.
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stormify-temporaryblobstorage into launchpad:master.
diff --git a/lib/canonical/launchpad/apidoc.tmp/index.html b/lib/canonical/launchpad/apidoc.tmp/index.html
deleted file mode 100644
index e69de29..0000000
--- a/lib/canonical/launchpad/apidoc.tmp/index.html
+++ /dev/null
diff --git a/lib/lp/bugs/browser/bugtarget.py b/lib/lp/bugs/browser/bugtarget.py
index 01b323d..45f45fa 100644
--- a/lib/lp/bugs/browser/bugtarget.py
+++ b/lib/lp/bugs/browser/bugtarget.py
@@ -34,7 +34,6 @@ from six.moves.urllib.parse import (
quote,
urlencode,
)
-from sqlobject import SQLObjectNotFound
from zope.browserpage import ViewPageTemplateFile
from zope.component import getUtility
from zope.formlib.form import Fields
@@ -841,7 +840,7 @@ class FileBugViewBase(LaunchpadFormView):
try:
return getUtility(IProcessApportBlobJobSource).getByBlobUUID(
self.extra_data_token)
- except SQLObjectNotFound:
+ except NotFoundError:
return None
@property
diff --git a/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py b/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
index d96ddd4..d3efbe2 100644
--- a/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
+++ b/lib/lp/bugs/browser/tests/test_bugtarget_filebug.py
@@ -1,6 +1,8 @@
# Copyright 2010-2019 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
+from __future__ import absolute_import, print_function, unicode_literals
+
__metaclass__ = type
from textwrap import dedent
@@ -524,7 +526,7 @@ class FileBugViewBaseExtraDataTestCase(FileBugViewMixin, TestCaseWithFactory):
--boundary--
""" % command
- extra_data = dedent(raw_data)
+ extra_data = dedent(raw_data).encode("UTF-8")
temp_storage_manager = getUtility(ITemporaryStorageManager)
token = temp_storage_manager.new(extra_data)
transaction.commit()
@@ -687,7 +689,7 @@ class FileBugViewBaseExtraDataTestCase(FileBugViewMixin, TestCaseWithFactory):
self.assertEqual(
'text/plain; charset=utf-8', attachment.libraryfile.mimetype)
self.assertEqual(
- 'This is an attachment.\n\n', attachment.libraryfile.read())
+ b'This is an attachment.\n\n', attachment.libraryfile.read())
self.assertEqual(2, bug.messages.count())
self.assertEqual(2, len(bug.messages[1].bugattachments))
notifications = [
diff --git a/lib/lp/bugs/model/apportjob.py b/lib/lp/bugs/model/apportjob.py
index b99e3eb..a2f16e0 100644
--- a/lib/lp/bugs/model/apportjob.py
+++ b/lib/lp/bugs/model/apportjob.py
@@ -14,7 +14,6 @@ from cStringIO import StringIO
from lazr.delegates import delegate_to
import simplejson
import six
-from sqlobject import SQLObjectNotFound
from storm.expr import And
from storm.locals import (
Int,
@@ -27,6 +26,7 @@ from zope.interface import (
provider,
)
+from lp.app.errors import NotFoundError
from lp.bugs.interfaces.apportjob import (
ApportJobType,
IApportJob,
@@ -104,7 +104,7 @@ class ApportJob(StormBase):
"""Return the instance of this class whose key is supplied."""
instance = IStore(cls).get(cls, key)
if instance is None:
- raise SQLObjectNotFound(
+ raise NotFoundError(
'No occurrence of %s has key %s' % (cls.__name__, key))
return instance
@@ -136,12 +136,12 @@ class ApportJobDerived(
:return: the ApportJob with the specified id, as the current
ApportJobDerived subclass.
- :raises: SQLObjectNotFound if there is no job with the specified id,
+ :raises: NotFoundError if there is no job with the specified id,
or its job_type does not match the desired subclass.
"""
job = ApportJob.get(job_id)
if job.job_type != cls.class_job_type:
- raise SQLObjectNotFound(
+ raise NotFoundError(
'No object found with id %d and type %s' % (job_id,
cls.class_job_type.title))
return cls(job)
@@ -211,7 +211,7 @@ class ProcessApportBlobJob(ApportJobDerived):
job_for_blob = jobs_for_blob.one()
if job_for_blob is None:
- raise SQLObjectNotFound(
+ raise NotFoundError(
"No ProcessApportBlobJob found for UUID %s" % uuid)
return cls(job_for_blob)
diff --git a/lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt b/lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt
index 7d94d6f..3a42491 100644
--- a/lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt
+++ b/lib/lp/bugs/stories/guided-filebug/xx-bug-reporting-tools.txt
@@ -35,7 +35,8 @@ to give the data to the +filebug page.
To avoid having the tool from parsing the HTML page, the token is
returned as a X-Launchpad-Blob-Token header in the response as well:
- >>> blob_token = anon_browser.headers['X-Launchpad-Blob-Token']
+ >>> blob_token = six.ensure_text(
+ ... anon_browser.headers['X-Launchpad-Blob-Token'])
This token we can now pass to the +filebug page by appending it to the
URL as an extra path component, like '+filebug/12345abcde'.
@@ -162,7 +163,8 @@ that will be used to automatically fill in a suggested title.
>>> anon_browser.getControl(name='field.blob').add_file( # Don't change!
... extra_filebug_data_with_subject, 'not/important', 'not.important')
>>> anon_browser.getControl(name='FORM_SUBMIT').click() # Don't change!
- >>> blob_token = anon_browser.headers['X-Launchpad-Blob-Token']
+ >>> blob_token = six.ensure_text(
+ ... anon_browser.headers['X-Launchpad-Blob-Token'])
>>> process_blob(blob_token)
>>> user_browser.open(
@@ -191,7 +193,8 @@ initialized with that value.
>>> anon_browser.getControl(name='field.blob').add_file( # Don't change!
... extra_filebug_data_with_subject, 'not/important', 'not.important')
>>> anon_browser.getControl(name='FORM_SUBMIT').click() # Don't change!
- >>> blob_token = anon_browser.headers['X-Launchpad-Blob-Token']
+ >>> blob_token = six.ensure_text(
+ ... anon_browser.headers['X-Launchpad-Blob-Token'])
>>> process_blob(blob_token)
>>> user_browser.open(
@@ -264,7 +267,8 @@ Once we submit the bug report...
... extra_filebug_data_with_hwdb_submission, 'not/important',
... 'not.important')
>>> anon_browser.getControl(name='FORM_SUBMIT').click() # Don't change!
- >>> blob_token = anon_browser.headers['X-Launchpad-Blob-Token']
+ >>> blob_token = six.ensure_text(
+ ... anon_browser.headers['X-Launchpad-Blob-Token'])
>>> process_blob(blob_token)
>>> filebug_url = (
diff --git a/lib/lp/bugs/stories/guided-filebug/xx-ubuntu-filebug.txt b/lib/lp/bugs/stories/guided-filebug/xx-ubuntu-filebug.txt
index f614ab7..7e202d9 100644
--- a/lib/lp/bugs/stories/guided-filebug/xx-ubuntu-filebug.txt
+++ b/lib/lp/bugs/stories/guided-filebug/xx-ubuntu-filebug.txt
@@ -70,7 +70,8 @@ Filing bugs with Apport also allows us to get to the bug filing interface.
>>> anon_browser.getControl(name='field.blob').add_file(
... extra_filebug_data, 'not/important', 'not.important')
>>> anon_browser.getControl(name='FORM_SUBMIT').click()
- >>> blob_token = anon_browser.headers['X-Launchpad-Blob-Token']
+ >>> blob_token = six.ensure_text(
+ ... anon_browser.headers['X-Launchpad-Blob-Token'])
>>> from lp.bugs.interfaces.apportjob import IProcessApportBlobJobSource
>>> login('foo.bar@xxxxxxxxxxxxx')
>>> job = getUtility(IProcessApportBlobJobSource).getByBlobUUID(
diff --git a/lib/lp/bugs/tests/test_apportjob.py b/lib/lp/bugs/tests/test_apportjob.py
index c0b4799..9f5baa7 100644
--- a/lib/lp/bugs/tests/test_apportjob.py
+++ b/lib/lp/bugs/tests/test_apportjob.py
@@ -7,11 +7,11 @@ __metaclass__ = type
import os
-from sqlobject import SQLObjectNotFound
import transaction
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
+from lp.app.errors import NotFoundError
from lp.bugs.interfaces.apportjob import (
ApportJobType,
IApportJob,
@@ -221,10 +221,10 @@ class ProcessApportBlobJobTestCase(TestCaseWithFactory):
"match original BLOB.")
# If the UUID doesn't exist, getByBlobUUID() will raise a
- # SQLObjectNotFound error.
+ # NotFoundError.
self.assertRaises(
- SQLObjectNotFound,
- getUtility(IProcessApportBlobJobSource).getByBlobUUID, 'foobar')
+ NotFoundError,
+ getUtility(IProcessApportBlobJobSource).getByBlobUUID, u'foobar')
def test_create_job_creates_only_one(self):
# IProcessApportBlobJobSource.create() will create only one
diff --git a/lib/lp/services/temporaryblobstorage/doc/temporaryblobstorage.txt b/lib/lp/services/temporaryblobstorage/doc/temporaryblobstorage.txt
index f51b80a..463966f 100644
--- a/lib/lp/services/temporaryblobstorage/doc/temporaryblobstorage.txt
+++ b/lib/lp/services/temporaryblobstorage/doc/temporaryblobstorage.txt
@@ -19,7 +19,7 @@ perhaps as attachments.
To create a new TemporaryBlob, use ITemporaryStorageManager.new:
- >>> data = 'abcdefg'
+ >>> data = b'abcdefg'
>>> uuid = tsm.new(data)
>>> uuid is not None
True
@@ -33,12 +33,12 @@ same transaction as we stored it in:
To retrieve a blob, we can also use the tsm.
>>> blob = tsm.fetch(uuid)
- >>> blob.blob
- 'abcdefg'
+ >>> blob.blob == b'abcdefg'
+ True
We can delete a blob by UUID too:
- >>> print tsm.delete(uuid)
+ >>> print(tsm.delete(uuid))
None
Size limits can be enforced, although this is turned off by default:
@@ -67,9 +67,9 @@ ProcessApportBlobJob for the blob has been completed.
The hasBeenProcessed() of a newly created blob, with no
associated ProcessApportBlobJob, will return False.
- >>> blob_token = getUtility(ITemporaryStorageManager).new("Blob data")
+ >>> blob_token = getUtility(ITemporaryStorageManager).new(b"Blob data")
>>> blob = getUtility(ITemporaryStorageManager).fetch(blob_token)
- >>> print blob.hasBeenProcessed()
+ >>> print(blob.hasBeenProcessed())
False
We'll create a ProcessApportBlobJob for the blob.
@@ -86,19 +86,19 @@ We'll create a ProcessApportBlobJob for the blob.
Before the job is run, the blob's hasBeenProcessed() method will return
False.
- >>> print blob.hasBeenProcessed()
+ >>> print(blob.hasBeenProcessed())
False
Whilst the job is running, the blob's hasBeenProcessed() method will
return False.
>>> processing_job.job.start()
- >>> print blob.hasBeenProcessed()
+ >>> print(blob.hasBeenProcessed())
False
Once the job is complete, the blob's hasBeenProcessed() method will
return True.
>>> processing_job.job.complete()
- >>> print blob.hasBeenProcessed()
+ >>> print(blob.hasBeenProcessed())
True
diff --git a/lib/lp/services/temporaryblobstorage/model.py b/lib/lp/services/temporaryblobstorage/model.py
index b4e42e8..8fb48b0 100644
--- a/lib/lp/services/temporaryblobstorage/model.py
+++ b/lib/lp/services/temporaryblobstorage/model.py
@@ -13,18 +13,22 @@ from datetime import timedelta
from io import BytesIO
import uuid
-from sqlobject import (
- ForeignKey,
- SQLObjectNotFound,
- StringCol,
+import pytz
+import six
+from storm.locals import (
+ DateTime,
+ Int,
+ Reference,
+ Unicode,
)
from zope.component import getUtility
from zope.interface import implementer
+from lp.app.errors import NotFoundError
from lp.services.config import config
from lp.services.database.constants import DEFAULT
-from lp.services.database.datetimecol import UtcDateTimeCol
-from lp.services.database.sqlbase import SQLBase
+from lp.services.database.interfaces import IStore
+from lp.services.database.stormbase import StormBase
from lp.services.job.interfaces.job import JobStatus
from lp.services.librarian.interfaces import ILibraryFileAliasSet
from lp.services.temporaryblobstorage.interfaces import (
@@ -36,17 +40,21 @@ from lp.services.utils import utc_now
@implementer(ITemporaryBlobStorage)
-class TemporaryBlobStorage(SQLBase):
+class TemporaryBlobStorage(StormBase):
"""A temporary BLOB stored in Launchpad."""
- _table = 'TemporaryBlobStorage'
+ __storm_table__ = 'TemporaryBlobStorage'
- uuid = StringCol(notNull=True, alternateID=True)
- file_alias = ForeignKey(
- dbName='file_alias', foreignKey='LibraryFileAlias', notNull=True,
- alternateID=True
- )
- date_created = UtcDateTimeCol(notNull=True, default=DEFAULT)
+ id = Int(primary=True)
+
+ uuid = Unicode(allow_none=False)
+ file_alias_id = Int(name='file_alias', allow_none=False)
+ file_alias = Reference(file_alias_id, 'LibraryFileAlias.id')
+ date_created = DateTime(tzinfo=pytz.UTC, allow_none=False, default=DEFAULT)
+
+ def __init__(self, uuid, file_alias):
+ self.uuid = uuid
+ self.file_alias = file_alias
@property
def blob(self):
@@ -63,7 +71,7 @@ class TemporaryBlobStorage(SQLBase):
try:
job_for_blob = getUtility(
IProcessApportBlobJobSource).getByBlobUUID(self.uuid)
- except SQLObjectNotFound:
+ except NotFoundError:
return None
return job_for_blob
@@ -115,30 +123,33 @@ class TemporaryStorageManager:
# create the BLOB and return the UUID
- new_uuid = str(uuid.uuid1())
+ new_uuid = six.text_type(uuid.uuid1())
# We use a random filename, so only things that can look up the
# secret can retrieve the original data (which is why we don't use
# the UUID we return to the user as the filename, nor the filename
# of the object they uploaded).
- secret = str(uuid.uuid1())
+ secret = six.text_type(uuid.uuid1())
file_alias = getUtility(ILibraryFileAliasSet).create(
secret, len(blob), BytesIO(blob),
'application/octet-stream', expires
)
- TemporaryBlobStorage(uuid=new_uuid, file_alias=file_alias)
+ IStore(TemporaryBlobStorage).add(
+ TemporaryBlobStorage(uuid=new_uuid, file_alias=file_alias))
return new_uuid
def fetch(self, uuid):
"""See ITemporaryStorageManager."""
- return TemporaryBlobStorage.selectOneBy(uuid=uuid)
+ return IStore(TemporaryBlobStorage).find(
+ TemporaryBlobStorage, uuid=uuid).one()
def delete(self, uuid):
"""See ITemporaryStorageManager."""
- blob = TemporaryBlobStorage.selectOneBy(uuid=uuid)
+ store = IStore(TemporaryBlobStorage)
+ blob = store.find(TemporaryBlobStorage, uuid=uuid).one()
if blob is not None:
- TemporaryBlobStorage.delete(blob.id)
+ store.remove(blob)
def default_temporary_blob_storage_list(self):
"""See `ITemporaryStorageManager`."""
diff --git a/lib/lp/services/temporaryblobstorage/stories/xx-temporary-blob-storage.txt b/lib/lp/services/temporaryblobstorage/stories/xx-temporary-blob-storage.txt
index e3431d0..016fae4 100644
--- a/lib/lp/services/temporaryblobstorage/stories/xx-temporary-blob-storage.txt
+++ b/lib/lp/services/temporaryblobstorage/stories/xx-temporary-blob-storage.txt
@@ -20,10 +20,10 @@ If we add a new blob, it will not show up in the temporary_blobs entries set.
>>> testfiles = os.path.join(config.root,
... 'lib/lp/bugs/tests/testfiles')
- >>> blob_file = open(
- ... os.path.join(testfiles, 'extra_filebug_data.msg'))
- >>> blob_data = blob_file.read()
- >>> print blob_data[307:373]
+ >>> with open(os.path.join(testfiles, 'extra_filebug_data.msg'),
+ ... 'rb') as blob_file:
+ ... blob_data = blob_file.read()
+ >>> print(blob_data[307:373].decode('UTF-8'))
--boundary
Content-disposition: attachment; filename='attachment1'
@@ -63,15 +63,15 @@ It's possible to see whether a blob has been processed by calling its
hasBeenProcessed() method. In the case of the blob we created above, it
hasn't been processed because no job was created to process it.
- >>> print webservice.named_get(
- ... blob['self_link'], 'hasBeenProcessed').jsonBody()
+ >>> print(webservice.named_get(
+ ... blob['self_link'], 'hasBeenProcessed').jsonBody())
False
However, since the blob has not been processed there will be no
job processed data at this point.
- >>> print webservice.named_get(
- ... blob['self_link'], 'getProcessedData').jsonBody()
+ >>> print(webservice.named_get(
+ ... blob['self_link'], 'getProcessedData').jsonBody())
None
Once the blob has been processed, its hasBeenProcessed() method will
@@ -86,8 +86,8 @@ return True.
>>> job.run()
>>> logout()
- >>> print webservice.named_get(
- ... blob['self_link'], 'hasBeenProcessed').jsonBody()
+ >>> print(webservice.named_get(
+ ... blob['self_link'], 'hasBeenProcessed').jsonBody())
True
And now the blob's parsed-out metadata is now accessible.
@@ -95,12 +95,12 @@ And now the blob's parsed-out metadata is now accessible.
>>> metadata = webservice.named_get(
... blob['self_link'], 'getProcessedData').jsonBody()
- >>> print metadata['extra_description']
+ >>> print(metadata['extra_description'])
This should be added to the description.
- >>> print len(metadata['comments'])
+ >>> print(len(metadata['comments']))
2
>>> attachment = metadata['attachments'][0]
- >>> print attachment['description']
+ >>> print(attachment['description'])
attachment1
diff --git a/lib/lp/services/temporaryblobstorage/stories/xx-tempstorage.txt b/lib/lp/services/temporaryblobstorage/stories/xx-tempstorage.txt
index c683501..c75a4d6 100644
--- a/lib/lp/services/temporaryblobstorage/stories/xx-tempstorage.txt
+++ b/lib/lp/services/temporaryblobstorage/stories/xx-tempstorage.txt
@@ -1,4 +1,3 @@
-
It is possible for anybody to upload a BLOB to Launchpad which will be
stored for a short period of time, and deleted if unused.
@@ -21,7 +20,7 @@ middle of the data so we can ensure binary data is handled correctly.
... 'Your ticket is "([\w-]+)"', anon_browser.contents)
>>> match is not None
True
- >>> ticket = match.group(1)
+ >>> ticket = six.ensure_text(match.group(1))
For easy access to the token in scripts, it's also stored in a HTTP
header in the response: X-Launchpad-Blob-Token
@@ -33,10 +32,11 @@ Retrieve the blob and make sure it got stored correctly.
>>> from lp.testing import login, logout, ANONYMOUS
>>> login(ANONYMOUS)
- >>> from lp.services.temporaryblobstorage.model import (
- ... TemporaryBlobStorage)
- >>> blob = TemporaryBlobStorage.byUuid(ticket)
- >>> blob.blob
- 'abcd\x00efg'
+ >>> from zope.component import getUtility
+ >>> from lp.services.temporaryblobstorage.interfaces import (
+ ... ITemporaryStorageManager,
+ ... )
+ >>> blob = getUtility(ITemporaryStorageManager).fetch(ticket)
+ >>> blob.blob == b'abcd\x00efg'
+ True
>>> logout()
-
diff --git a/lib/lp/services/temporaryblobstorage/tests/test_doc.py b/lib/lp/services/temporaryblobstorage/tests/test_doc.py
index 24de8de..f22077e 100644
--- a/lib/lp/services/temporaryblobstorage/tests/test_doc.py
+++ b/lib/lp/services/temporaryblobstorage/tests/test_doc.py
@@ -9,6 +9,8 @@ import os
from lp.services.testing import build_test_suite
from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.pages import setUpGlobs
+from lp.testing.systemdocs import setUp
here = os.path.dirname(os.path.realpath(__file__))
@@ -17,4 +19,7 @@ special = {}
def test_suite():
- return build_test_suite(here, special, layer=LaunchpadFunctionalLayer)
+ return build_test_suite(
+ here, special, layer=LaunchpadFunctionalLayer,
+ setUp=lambda test: setUp(test, future=True),
+ pageTestsSetUp=lambda test: setUpGlobs(test, future=True))
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index ac23612..a7d3cf1 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -4470,9 +4470,10 @@ class BareLaunchpadObjectFactory(ObjectFactory):
if blob_file is not None:
blob_path = os.path.join(
config.root, 'lib/lp/bugs/tests/testfiles', blob_file)
- blob = open(blob_path).read()
+ with open(blob_path, 'rb') as blob_file:
+ blob = blob_file.read()
if blob is None:
- blob = self.getUniqueString()
+ blob = self.getUniqueBytes()
new_uuid = getUtility(ITemporaryStorageManager).new(blob, expires)
return getUtility(ITemporaryStorageManager).fetch(new_uuid)
@@ -4483,7 +4484,8 @@ class BareLaunchpadObjectFactory(ObjectFactory):
It doesn't actually run the job. It fakes it, and uses a fake
librarian file so as to work without the librarian.
"""
- blob = TemporaryBlobStorage(uuid=str(uuid.uuid1()), file_alias=1)
+ blob = TemporaryBlobStorage(
+ uuid=six.text_type(uuid.uuid1()), file_alias=1)
job = getUtility(IProcessApportBlobJobSource).create(blob)
job.job.start()
removeSecurityProxy(job).metadata = {
