launchpad-reviewers team mailing list archive

[Colin Watson <cjwatson@xxxxxxxxxxxxx>]

Colin Watson has proposed merging ~cjwatson/launchpad:stormify-codeimportresult into launchpad:master.

Commit message:
Convert CodeImportResult to Storm

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/+git/launchpad/+merge/384470
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of ~cjwatson/launchpad:stormify-codeimportresult into launchpad:master.

diff --git a/lib/lp/code/model/codeimport.py b/lib/lp/code/model/codeimport.py
index 4162632..b6eb130 100644
--- a/lib/lp/code/model/codeimport.py
+++ b/lib/lp/code/model/codeimport.py
@@ -16,7 +16,6 @@ from lazr.lifecycle.event import ObjectCreatedEvent
 from sqlobject import (
     ForeignKey,
     IntervalCol,
-    SQLMultipleJoin,
     SQLObjectNotFound,
     StringCol,
     )
@@ -28,9 +27,10 @@ from storm.expr import (
     )
 from storm.locals import (
     Int,
+    Reference,
+    ReferenceSet,
     Store,
     )
-from storm.references import Reference
 from zope.component import getUtility
 from zope.event import notify
 from zope.interface import implementer
@@ -186,9 +186,9 @@ class CodeImport(SQLBase):
             if job.state == CodeImportJobState.PENDING:
                 CodeImportJobWorkflow().deletePendingJob(self)
 
-    results = SQLMultipleJoin(
-        'CodeImportResult', joinColumn='code_import',
-        orderBy=['-date_job_started'])
+    results = ReferenceSet(
+        '<primary key>', 'CodeImportResult.code_import_id',
+        order_by=Desc('CodeImportResult.date_job_started'))
 
     @property
     def consecutive_failure_count(self):
diff --git a/lib/lp/code/model/codeimportresult.py b/lib/lp/code/model/codeimportresult.py
index c0caa47..6c1a81f 100644
--- a/lib/lp/code/model/codeimportresult.py
+++ b/lib/lp/code/model/codeimportresult.py
@@ -1,14 +1,19 @@
-# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Database classes for the CodeImportResult table."""
 
+from __future__ import absolute_import, print_function, unicode_literals
+
 __metaclass__ = type
 __all__ = ['CodeImportResult', 'CodeImportResultSet']
 
-from sqlobject import (
-    ForeignKey,
-    StringCol,
+import pytz
+from storm.locals import (
+    DateTime,
+    Int,
+    Reference,
+    Unicode,
     )
 from zope.interface import implementer
 
@@ -19,36 +24,53 @@ from lp.code.interfaces.codeimportresult import (
     )
 from lp.registry.interfaces.person import validate_public_person
 from lp.services.database.constants import UTC_NOW
-from lp.services.database.datetimecol import UtcDateTimeCol
-from lp.services.database.enumcol import EnumCol
-from lp.services.database.sqlbase import SQLBase
+from lp.services.database.enumcol import DBEnum
+from lp.services.database.interfaces import IStore
+from lp.services.database.stormbase import StormBase
 
 
 @implementer(ICodeImportResult)
-class CodeImportResult(SQLBase):
+class CodeImportResult(StormBase):
     """See `ICodeImportResult`."""
 
-    date_created = UtcDateTimeCol(notNull=True, default=UTC_NOW)
+    __storm_table__ = 'CodeImportResult'
+
+    id = Int(primary=True)
+
+    date_created = DateTime(tzinfo=pytz.UTC, allow_none=False, default=UTC_NOW)
+
+    code_import_id = Int(name='code_import', allow_none=False)
+    code_import = Reference(code_import_id, 'CodeImport.id')
 
-    code_import = ForeignKey(
-        dbName='code_import', foreignKey='CodeImport', notNull=True)
+    machine_id = Int(name='machine', allow_none=False)
+    machine = Reference(machine_id, 'CodeImportMachine.id')
 
-    machine = ForeignKey(
-        dbName='machine', foreignKey='CodeImportMachine', notNull=True)
+    requesting_user_id = Int(
+        name='requesting_user', allow_none=True,
+        validator=validate_public_person, default=None)
+    requesting_user = Reference(requesting_user_id, 'Person.id')
 
-    requesting_user = ForeignKey(
-        dbName='requesting_user', foreignKey='Person',
-        storm_validator=validate_public_person, default=None)
+    log_excerpt = Unicode(allow_none=True, default=None)
 
-    log_excerpt = StringCol(default=None)
+    log_file_id = Int(name='log_file', allow_none=True, default=None)
+    log_file = Reference(log_file_id, 'LibraryFileAlias.id')
 
-    log_file = ForeignKey(
-        dbName='log_file', foreignKey='LibraryFileAlias', default=None)
+    status = DBEnum(enum=CodeImportResultStatus, allow_none=False)
 
-    status = EnumCol(
-        enum=CodeImportResultStatus, notNull=True)
+    date_job_started = DateTime(tzinfo=pytz.UTC, allow_none=False)
 
-    date_job_started = UtcDateTimeCol(notNull=True)
+    def __init__(self, code_import, machine, status, date_job_started,
+                 requesting_user=None, log_excerpt=None, log_file=None,
+                 date_created=UTC_NOW):
+        super(CodeImportResult, self).__init__()
+        self.code_import = code_import
+        self.machine = machine
+        self.status = status
+        self.date_job_started = date_job_started
+        self.requesting_user = requesting_user
+        self.log_excerpt = log_excerpt
+        self.log_file = log_file
+        self.date_created = date_created
 
     @property
     def date_job_finished(self):
@@ -69,8 +91,10 @@ class CodeImportResultSet(object):
         """See `ICodeImportResultSet`."""
         if date_job_finished is None:
             date_job_finished = UTC_NOW
-        return CodeImportResult(
+        result = CodeImportResult(
             code_import=code_import, machine=machine,
             requesting_user=requesting_user, log_excerpt=log_excerpt,
             log_file=log_file, status=status,
             date_job_started=date_job_started, date_created=date_job_finished)
+        IStore(CodeImportResult).add(result)
+        return result
diff --git a/lib/lp/code/model/tests/test_codeimport.py b/lib/lp/code/model/tests/test_codeimport.py
index aca0a75..5943d2b 100644
--- a/lib/lp/code/model/tests/test_codeimport.py
+++ b/lib/lp/code/model/tests/test_codeimport.py
@@ -12,7 +12,6 @@ from datetime import (
 from functools import partial
 
 import pytz
-from sqlobject import SQLObjectNotFound
 from storm.store import Store
 from testscenarios import (
     load_tests_apply_scenarios,
@@ -357,11 +356,9 @@ class TestCodeImportDeletion(TestCodeImportBase):
             code_import=code_import)
         code_import_result_id = code_import_result.id
         CodeImportSet().delete(code_import_result.code_import)
-        # CodeImportResult.get should not raise anything.
-        # But since it populates the object cache, we must invalidate it.
-        Store.of(code_import_result).invalidate(code_import_result)
-        self.assertRaises(
-            SQLObjectNotFound, CodeImportResult.get, code_import_result_id)
+        store = Store.of(code_import_result)
+        store.invalidate(code_import_result)
+        self.assertIsNone(store.get(CodeImportResult, code_import_result_id))
 
 
 class TestCodeImportStatusUpdate(TestCodeImportBase):
diff --git a/lib/lp/code/model/tests/test_codeimportjob.py b/lib/lp/code/model/tests/test_codeimportjob.py
index 89ed0cc..2f343e8 100644
--- a/lib/lp/code/model/tests/test_codeimportjob.py
+++ b/lib/lp/code/model/tests/test_codeimportjob.py
@@ -947,7 +947,7 @@ class TestCodeImportJobWorkflowFinishJob(TestCaseWithFactory,
                 running_job, CodeImportResultStatus.FAILURE, None)
             intervals.append(
                 code_import.import_job.date_due -
-                code_import.results[-1].date_job_started)
+                code_import.results.last().date_job_started)
             running_job = code_import.import_job
             getUtility(ICodeImportJobWorkflow).startJob(
                 running_job, self.machine)
diff --git a/lib/lp/code/xmlrpc/tests/test_codeimportscheduler.py b/lib/lp/code/xmlrpc/tests/test_codeimportscheduler.py
index 91ceae3..c79f741 100644
--- a/lib/lp/code/xmlrpc/tests/test_codeimportscheduler.py
+++ b/lib/lp/code/xmlrpc/tests/test_codeimportscheduler.py
@@ -123,7 +123,7 @@ class TestCodeImportSchedulerAPI(TestCaseWithFactory):
             code_import_job.id, CodeImportResultStatus.SUCCESS.name,
             log_file_alias.http_url)
         self.assertEqual(
-            log_file_alias, code_import.results[-1].log_file)
+            log_file_alias, code_import.results.last().log_file)
 
     def test_finishJobID_not_found(self):
         # getImportDataForJobID returns a NoSuchCodeImportJob fault when there
diff --git a/lib/lp/scripts/tests/test_garbo.py b/lib/lp/scripts/tests/test_garbo.py
index dfeeba8..14ba31f 100644
--- a/lib/lp/scripts/tests/test_garbo.py
+++ b/lib/lp/scripts/tests/test_garbo.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2019 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test the database garbage collector."""
@@ -509,17 +509,17 @@ class TestGarbo(FakeAdapterMixin, TestCaseWithFactory):
             config.codeimport.consecutive_failure_limit - 1)
 
         switch_dbuser('testadmin')
-        code_import_id = self.factory.makeCodeImport().id
-        machine_id = self.factory.makeCodeImportMachine().id
-        requester_id = self.factory.makePerson().id
+        code_import = self.factory.makeCodeImport()
+        machine = self.factory.makeCodeImportMachine()
+        requester = self.factory.makePerson()
         transaction.commit()
 
         def new_code_import_result(timestamp):
             switch_dbuser('testadmin')
             CodeImportResult(
                 date_created=timestamp,
-                code_importID=code_import_id, machineID=machine_id,
-                requesting_userID=requester_id,
+                code_import=code_import, machine=machine,
+                requesting_user=requester,
                 status=CodeImportResultStatus.FAILURE,
                 date_job_started=timestamp)
             transaction.commit()
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 815ad8f..c04041d 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -2544,7 +2544,7 @@ class BareLaunchpadObjectFactory(ObjectFactory):
             machine = self.makeCodeImportMachine()
         requesting_user = None
         if log_excerpt is None:
-            log_excerpt = self.getUniqueString()
+            log_excerpt = self.getUniqueUnicode()
         if result_status is None:
             result_status = CodeImportResultStatus.FAILURE
         if date_finished is None: