Merge ~cjwatson/launchpad:stormify-branchmergeproposal into launchpad:master
- Git
- lp:~cjwatson/launchpad
- stormify-branchmergeproposal
- Merge into master
Proposed by
Colin Watson
Status: | Merged |
---|---|
Approved by: | Colin Watson |
Approved revision: | b6f4d067affc49737c6f4b0cbee3c8d55f56f1a5 |
Merge reported by: | Otto Co-Pilot |
Merged at revision: | not available |
Proposed branch: | ~cjwatson/launchpad:stormify-branchmergeproposal |
Merge into: | launchpad:master |
Diff against target: |
817 lines (+204/-142) 9 files modified
lib/lp/code/interfaces/branchmergeproposal.py (+4/-4) lib/lp/code/mail/codehandler.py (+5/-5) lib/lp/code/model/branch.py (+14/-10) lib/lp/code/model/branchcollection.py (+11/-7) lib/lp/code/model/branchmergeproposal.py (+138/-93) lib/lp/code/model/gitcollection.py (+6/-5) lib/lp/code/model/gitrepository.py (+5/-5) lib/lp/code/model/tests/test_branch.py (+9/-6) lib/lp/code/model/tests/test_gitrepository.py (+12/-7) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Guruprasad | Approve | ||
Review via email:
|
Commit message
Convert BranchMergeProposal to Storm
Description of the change
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | diff --git a/lib/lp/code/interfaces/branchmergeproposal.py b/lib/lp/code/interfaces/branchmergeproposal.py |
2 | index 20b4ceb..e65eef6 100644 |
3 | --- a/lib/lp/code/interfaces/branchmergeproposal.py |
4 | +++ b/lib/lp/code/interfaces/branchmergeproposal.py |
5 | @@ -93,16 +93,16 @@ class IBranchMergeProposalPublic(IPrivacy): |
6 | readonly=True, |
7 | description=_("The tracking number for this merge proposal."), |
8 | ) |
9 | - source_branchID = Int( |
10 | + source_branch_id = Int( |
11 | title=_("Source branch ID"), required=False, readonly=True |
12 | ) |
13 | - source_git_repositoryID = Int( |
14 | + source_git_repository_id = Int( |
15 | title=_("Source Git repository ID"), required=False, readonly=True |
16 | ) |
17 | - prerequisite_branchID = Int( |
18 | + prerequisite_branch_id = Int( |
19 | title=_("Prerequisite branch ID"), required=False, readonly=True |
20 | ) |
21 | - prerequisite_git_repositoryID = Int( |
22 | + prerequisite_git_repository_id = Int( |
23 | title=_("Prerequisite Git repository ID"), |
24 | required=False, |
25 | readonly=True, |
26 | diff --git a/lib/lp/code/mail/codehandler.py b/lib/lp/code/mail/codehandler.py |
27 | index 7a8a3f0..6eb574a 100644 |
28 | --- a/lib/lp/code/mail/codehandler.py |
29 | +++ b/lib/lp/code/mail/codehandler.py |
30 | @@ -10,12 +10,12 @@ from zope.component import getUtility |
31 | from zope.interface import implementer |
32 | from zope.security.interfaces import Unauthorized |
33 | |
34 | +from lp.app.errors import NotFoundError |
35 | from lp.code.adapters.branch import BranchMergeProposalNoPreviewDiffDelta |
36 | from lp.code.enums import CodeReviewVote |
37 | from lp.code.errors import UserNotBranchReviewer |
38 | from lp.code.interfaces.branchmergeproposal import IBranchMergeProposalGetter |
39 | from lp.services.config import config |
40 | -from lp.services.database.sqlobject import SQLObjectNotFound |
41 | from lp.services.mail.commands import EmailCommand, EmailCommandCollection |
42 | from lp.services.mail.helpers import ( |
43 | IncomingEmailError, |
44 | @@ -42,7 +42,7 @@ class InvalidBranchMergeProposalAddress(BadBranchMergeProposalAddress): |
45 | """The user-supplied address is not an acceptable value.""" |
46 | |
47 | |
48 | -class NonExistantBranchMergeProposalAddress(BadBranchMergeProposalAddress): |
49 | +class NonExistentBranchMergeProposalAddress(BadBranchMergeProposalAddress): |
50 | """The BranchMergeProposal specified by the address does not exist.""" |
51 | |
52 | |
53 | @@ -310,7 +310,7 @@ class CodeHandler: |
54 | user = getUtility(ILaunchBag).user |
55 | try: |
56 | merge_proposal = self.getBranchMergeProposal(email_addr) |
57 | - except NonExistantBranchMergeProposalAddress: |
58 | + except NonExistentBranchMergeProposalAddress: |
59 | send_process_error_notification( |
60 | str(user.preferredemail.email), |
61 | "Submit Request Failure", |
62 | @@ -375,5 +375,5 @@ class CodeHandler: |
63 | getter = getUtility(IBranchMergeProposalGetter) |
64 | try: |
65 | return getter.get(merge_proposal_id) |
66 | - except SQLObjectNotFound: |
67 | - raise NonExistantBranchMergeProposalAddress(email_addr) |
68 | + except NotFoundError: |
69 | + raise NonExistentBranchMergeProposalAddress(email_addr) |
70 | diff --git a/lib/lp/code/model/branch.py b/lib/lp/code/model/branch.py |
71 | index b9cfedc..b9f9b5f 100644 |
72 | --- a/lib/lp/code/model/branch.py |
73 | +++ b/lib/lp/code/model/branch.py |
74 | @@ -134,7 +134,7 @@ from lp.services.database.datetimecol import UtcDateTimeCol |
75 | from lp.services.database.decoratedresultset import DecoratedResultSet |
76 | from lp.services.database.enumcol import DBEnum |
77 | from lp.services.database.interfaces import IPrimaryStore, IStore |
78 | -from lp.services.database.sqlbase import SQLBase, sqlvalues |
79 | +from lp.services.database.sqlbase import SQLBase |
80 | from lp.services.database.sqlobject import ForeignKey, IntCol, StringCol |
81 | from lp.services.database.stormexpr import Array, ArrayAgg, ArrayIntersects |
82 | from lp.services.helpers import shortlist |
83 | @@ -559,12 +559,14 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin): |
84 | @property |
85 | def dependent_branches(self): |
86 | """See `IBranch`.""" |
87 | - return BranchMergeProposal.select( |
88 | - """ |
89 | - BranchMergeProposal.dependent_branch = %s AND |
90 | - BranchMergeProposal.queue_status NOT IN %s |
91 | - """ |
92 | - % sqlvalues(self, BRANCH_MERGE_PROPOSAL_FINAL_STATES) |
93 | + return Store.of(self).find( |
94 | + BranchMergeProposal, |
95 | + BranchMergeProposal.prerequisite_branch == self, |
96 | + Not( |
97 | + BranchMergeProposal.queue_status.is_in( |
98 | + BRANCH_MERGE_PROPOSAL_FINAL_STATES |
99 | + ) |
100 | + ), |
101 | ) |
102 | |
103 | def getMergeProposals( |
104 | @@ -942,7 +944,9 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin): |
105 | ) |
106 | # Cannot use self.landing_candidates, because it ignores merged |
107 | # merge proposals. |
108 | - for merge_proposal in BranchMergeProposal.selectBy(target_branch=self): |
109 | + for merge_proposal in Store.of(self).find( |
110 | + BranchMergeProposal, target_branch=self |
111 | + ): |
112 | deletion_operations.append( |
113 | DeletionCallable( |
114 | merge_proposal, |
115 | @@ -953,8 +957,8 @@ class Branch(SQLBase, WebhookTargetMixin, BzrIdentityMixin): |
116 | merge_proposal.deleteProposal, |
117 | ) |
118 | ) |
119 | - for merge_proposal in BranchMergeProposal.selectBy( |
120 | - prerequisite_branch=self |
121 | + for merge_proposal in Store.of(self).find( |
122 | + BranchMergeProposal, prerequisite_branch=self |
123 | ): |
124 | alteration_operations.append(ClearDependentBranch(merge_proposal)) |
125 | |
126 | diff --git a/lib/lp/code/model/branchcollection.py b/lib/lp/code/model/branchcollection.py |
127 | index 8ffbb63..55cbb0c 100644 |
128 | --- a/lib/lp/code/model/branchcollection.py |
129 | +++ b/lib/lp/code/model/branchcollection.py |
130 | @@ -431,14 +431,16 @@ class GenericBranchCollection: |
131 | Join( |
132 | BranchMergeProposal, |
133 | And( |
134 | - Branch.id == BranchMergeProposal.source_branchID, |
135 | + Branch.id == BranchMergeProposal.source_branch_id, |
136 | *( |
137 | self._branch_filter_expressions |
138 | + self._asymmetric_filter_expressions |
139 | ), |
140 | ), |
141 | ), |
142 | - Join(Target, Target.id == BranchMergeProposal.target_branchID), |
143 | + Join( |
144 | + Target, Target.id == BranchMergeProposal.target_branch_id |
145 | + ), |
146 | ] |
147 | ) |
148 | expressions = self._getBranchVisibilityExpression() |
149 | @@ -446,7 +448,7 @@ class GenericBranchCollection: |
150 | if for_branches is not None: |
151 | branch_ids = [branch.id for branch in for_branches] |
152 | expressions.append( |
153 | - BranchMergeProposal.source_branchID.is_in(branch_ids) |
154 | + BranchMergeProposal.source_branch_id.is_in(branch_ids) |
155 | ) |
156 | if target_branch is not None: |
157 | expressions.append( |
158 | @@ -467,7 +469,7 @@ class GenericBranchCollection: |
159 | == BranchRevision.sequence, |
160 | BranchRevision.revision_id == Revision.id, |
161 | BranchRevision.branch_id |
162 | - == BranchMergeProposal.target_branchID, |
163 | + == BranchMergeProposal.target_branch_id, |
164 | Revision.revision_id == merged_revision, |
165 | ] |
166 | ) |
167 | @@ -512,7 +514,7 @@ class GenericBranchCollection: |
168 | # Need to filter on Branch beyond the with constraints. |
169 | expressions += self._asymmetric_filter_expressions |
170 | expressions.append( |
171 | - BranchMergeProposal.source_branchID == Branch.id |
172 | + BranchMergeProposal.source_branch_id == Branch.id |
173 | ) |
174 | tables.append(Branch) |
175 | tables.extend(self._asymmetric_tables.values()) |
176 | @@ -568,12 +570,14 @@ class GenericBranchCollection: |
177 | |
178 | expressions = [ |
179 | CodeReviewVoteReference.reviewer == reviewer, |
180 | - BranchMergeProposal.source_branchID.is_in(self._getBranchSelect()), |
181 | + BranchMergeProposal.source_branch_id.is_in( |
182 | + self._getBranchSelect() |
183 | + ), |
184 | ] |
185 | visibility = self._getBranchVisibilityExpression() |
186 | if visibility: |
187 | expressions.append( |
188 | - BranchMergeProposal.target_branchID.is_in( |
189 | + BranchMergeProposal.target_branch_id.is_in( |
190 | Select(Branch.id, visibility) |
191 | ) |
192 | ) |
193 | diff --git a/lib/lp/code/model/branchmergeproposal.py b/lib/lp/code/model/branchmergeproposal.py |
194 | index bf927c8..7fb2176 100644 |
195 | --- a/lib/lp/code/model/branchmergeproposal.py |
196 | +++ b/lib/lp/code/model/branchmergeproposal.py |
197 | @@ -11,12 +11,13 @@ __all__ = [ |
198 | |
199 | import sys |
200 | from collections import defaultdict |
201 | +from datetime import timezone |
202 | from email.utils import make_msgid |
203 | from operator import attrgetter |
204 | |
205 | from lazr.lifecycle.event import ObjectCreatedEvent, ObjectDeletedEvent |
206 | from storm.expr import SQL, And, Desc, Join, LeftJoin, Not, Or, Select |
207 | -from storm.locals import Int, Reference |
208 | +from storm.locals import DateTime, Int, Reference, Unicode |
209 | from storm.store import Store |
210 | from zope.component import getUtility |
211 | from zope.error.interfaces import IErrorReportingUtility |
212 | @@ -82,11 +83,10 @@ from lp.registry.model.person import Person |
213 | from lp.services.config import config |
214 | from lp.services.database.bulk import load, load_referencing, load_related |
215 | from lp.services.database.constants import DEFAULT, UTC_NOW |
216 | -from lp.services.database.datetimecol import UtcDateTimeCol |
217 | from lp.services.database.enumcol import DBEnum |
218 | from lp.services.database.interfaces import IPrimaryStore, IStore |
219 | -from lp.services.database.sqlbase import SQLBase, quote |
220 | -from lp.services.database.sqlobject import ForeignKey, IntCol, StringCol |
221 | +from lp.services.database.sqlbase import quote |
222 | +from lp.services.database.stormbase import StormBase |
223 | from lp.services.helpers import shortlist |
224 | from lp.services.job.interfaces.job import JobStatus |
225 | from lp.services.job.model.job import Job |
226 | @@ -160,65 +160,62 @@ class TooManyRelatedBugs(Exception): |
227 | |
228 | |
229 | @implementer(IBranchMergeProposal, IHasBranchTarget) |
230 | -class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
231 | +class BranchMergeProposal(StormBase, BugLinkTargetMixin): |
232 | """A relationship between a person and a branch.""" |
233 | |
234 | - _table = "BranchMergeProposal" |
235 | - _defaultOrder = ["-date_created", "id"] |
236 | + __storm_table__ = "BranchMergeProposal" |
237 | + __storm_order__ = ("-date_created", "id") |
238 | |
239 | - registrant = ForeignKey( |
240 | - dbName="registrant", |
241 | - foreignKey="Person", |
242 | - storm_validator=validate_public_person, |
243 | - notNull=True, |
244 | - ) |
245 | + id = Int(primary=True) |
246 | |
247 | - source_branch = ForeignKey( |
248 | - dbName="source_branch", foreignKey="Branch", notNull=False |
249 | + registrant_id = Int( |
250 | + name="registrant", validator=validate_public_person, allow_none=False |
251 | ) |
252 | - source_git_repositoryID = Int( |
253 | + registrant = Reference(registrant_id, "Person.id") |
254 | + |
255 | + source_branch_id = Int(name="source_branch", allow_none=True) |
256 | + source_branch = Reference(source_branch_id, "Branch.id") |
257 | + source_git_repository_id = Int( |
258 | name="source_git_repository", allow_none=True |
259 | ) |
260 | source_git_repository = Reference( |
261 | - source_git_repositoryID, "GitRepository.id" |
262 | + source_git_repository_id, "GitRepository.id" |
263 | ) |
264 | - source_git_path = StringCol( |
265 | - dbName="source_git_path", default=None, notNull=False |
266 | + source_git_path = Unicode( |
267 | + name="source_git_path", default=None, allow_none=True |
268 | ) |
269 | - source_git_commit_sha1 = StringCol( |
270 | - dbName="source_git_commit_sha1", default=None, notNull=False |
271 | + source_git_commit_sha1 = Unicode( |
272 | + name="source_git_commit_sha1", default=None, allow_none=True |
273 | ) |
274 | |
275 | - target_branch = ForeignKey( |
276 | - dbName="target_branch", foreignKey="Branch", notNull=False |
277 | - ) |
278 | - target_git_repositoryID = Int( |
279 | + target_branch_id = Int(name="target_branch", allow_none=True) |
280 | + target_branch = Reference(target_branch_id, "Branch.id") |
281 | + target_git_repository_id = Int( |
282 | name="target_git_repository", allow_none=True |
283 | ) |
284 | target_git_repository = Reference( |
285 | - target_git_repositoryID, "GitRepository.id" |
286 | + target_git_repository_id, "GitRepository.id" |
287 | ) |
288 | - target_git_path = StringCol( |
289 | - dbName="target_git_path", default=None, notNull=False |
290 | + target_git_path = Unicode( |
291 | + name="target_git_path", default=None, allow_none=True |
292 | ) |
293 | - target_git_commit_sha1 = StringCol( |
294 | - dbName="target_git_commit_sha1", default=None, notNull=False |
295 | + target_git_commit_sha1 = Unicode( |
296 | + name="target_git_commit_sha1", default=None, allow_none=True |
297 | ) |
298 | |
299 | - prerequisite_branch = ForeignKey( |
300 | - dbName="dependent_branch", foreignKey="Branch", notNull=False |
301 | - ) |
302 | - prerequisite_git_repositoryID = Int( |
303 | + prerequisite_branch_id = Int(name="dependent_branch", allow_none=True) |
304 | + prerequisite_branch = Reference(prerequisite_branch_id, "Branch.id") |
305 | + prerequisite_git_repository_id = Int( |
306 | name="dependent_git_repository", allow_none=True |
307 | ) |
308 | prerequisite_git_repository = Reference( |
309 | - prerequisite_git_repositoryID, "GitRepository.id" |
310 | + prerequisite_git_repository_id, "GitRepository.id" |
311 | ) |
312 | - prerequisite_git_path = StringCol( |
313 | - dbName="dependent_git_path", default=None, notNull=False |
314 | + prerequisite_git_path = Unicode( |
315 | + name="dependent_git_path", default=None, allow_none=True |
316 | ) |
317 | - prerequisite_git_commit_sha1 = StringCol( |
318 | - dbName="dependent_git_commit_sha1", default=None, notNull=False |
319 | + prerequisite_git_commit_sha1 = Unicode( |
320 | + name="dependent_git_commit_sha1", default=None, allow_none=True |
321 | ) |
322 | |
323 | @property |
324 | @@ -303,9 +300,9 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
325 | else: |
326 | return self.source_git_repository |
327 | |
328 | - description = StringCol(default=None) |
329 | + description = Unicode(default=None) |
330 | |
331 | - whiteboard = StringCol(default=None) |
332 | + whiteboard = Unicode(default=None) |
333 | |
334 | queue_status = DBEnum( |
335 | enum=BranchMergeProposalStatus, |
336 | @@ -326,13 +323,13 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
337 | for obj in objects |
338 | ) |
339 | |
340 | - reviewer = ForeignKey( |
341 | - dbName="reviewer", |
342 | - foreignKey="Person", |
343 | - storm_validator=validate_person, |
344 | - notNull=False, |
345 | + reviewer_id = Int( |
346 | + name="reviewer", |
347 | + validator=validate_person, |
348 | + allow_none=True, |
349 | default=None, |
350 | ) |
351 | + reviewer = Reference(reviewer_id, "Person.id") |
352 | |
353 | @property |
354 | def next_preview_diff_job(self): |
355 | @@ -356,13 +353,13 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
356 | else: |
357 | return None |
358 | |
359 | - reviewed_revision_id = StringCol(default=None) |
360 | + reviewed_revision_id = Unicode(default=None) |
361 | |
362 | - commit_message = StringCol(default=None) |
363 | + commit_message = Unicode(default=None) |
364 | |
365 | - date_merged = UtcDateTimeCol(default=None) |
366 | - merged_revno = IntCol(default=None) |
367 | - merged_revision_id = StringCol(default=None) |
368 | + date_merged = DateTime(default=None, tzinfo=timezone.utc) |
369 | + merged_revno = Int(default=None) |
370 | + merged_revision_id = Unicode(default=None) |
371 | |
372 | @property |
373 | def merged_revision(self): |
374 | @@ -372,13 +369,56 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
375 | else: |
376 | return self.merged_revision_id |
377 | |
378 | - merge_reporter = ForeignKey( |
379 | - dbName="merge_reporter", |
380 | - foreignKey="Person", |
381 | - storm_validator=validate_public_person, |
382 | - notNull=False, |
383 | + merge_reporter_id = Int( |
384 | + name="merge_reporter", |
385 | + validator=validate_public_person, |
386 | + allow_none=True, |
387 | default=None, |
388 | ) |
389 | + merge_reporter = Reference(merge_reporter_id, "Person.id") |
390 | + |
391 | + def __init__( |
392 | + self, |
393 | + registrant, |
394 | + source_branch=None, |
395 | + source_git_repository=None, |
396 | + source_git_path=None, |
397 | + source_git_commit_sha1=None, |
398 | + target_branch=None, |
399 | + target_git_repository=None, |
400 | + target_git_path=None, |
401 | + target_git_commit_sha1=None, |
402 | + prerequisite_branch=None, |
403 | + prerequisite_git_repository=None, |
404 | + prerequisite_git_path=None, |
405 | + prerequisite_git_commit_sha1=None, |
406 | + description=None, |
407 | + whiteboard=None, |
408 | + queue_status=DEFAULT, |
409 | + commit_message=None, |
410 | + date_created=DEFAULT, |
411 | + date_review_requested=None, |
412 | + ): |
413 | + super().__init__() |
414 | + self.registrant = registrant |
415 | + self.source_branch = source_branch |
416 | + self.source_git_repository = source_git_repository |
417 | + self.source_git_path = source_git_path |
418 | + self.source_git_commit_sha1 = source_git_commit_sha1 |
419 | + self.target_branch = target_branch |
420 | + self.target_git_repository = target_git_repository |
421 | + self.target_git_path = target_git_path |
422 | + self.target_git_commit_sha1 = target_git_commit_sha1 |
423 | + self.prerequisite_branch = prerequisite_branch |
424 | + self.prerequisite_git_repository = prerequisite_git_repository |
425 | + self.prerequisite_git_path = prerequisite_git_path |
426 | + self.prerequisite_git_commit_sha1 = prerequisite_git_commit_sha1 |
427 | + self.description = description |
428 | + self.whiteboard = whiteboard |
429 | + self.queue_status = queue_status |
430 | + self.commit_message = commit_message |
431 | + self.date_created = date_created |
432 | + self.date_review_requested = date_review_requested |
433 | |
434 | @property |
435 | def bugs(self): |
436 | @@ -557,22 +597,24 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
437 | def address(self): |
438 | return "mp+%d@%s" % (self.id, config.launchpad.code_domain) |
439 | |
440 | - superseded_by = ForeignKey( |
441 | - dbName="superseded_by", |
442 | - foreignKey="BranchMergeProposal", |
443 | - notNull=False, |
444 | - default=None, |
445 | - ) |
446 | + superseded_by_id = Int(name="superseded_by", allow_none=True, default=None) |
447 | + superseded_by = Reference(superseded_by_id, "BranchMergeProposal.id") |
448 | |
449 | - _supersedes = Reference("<primary key>", "superseded_by", on_remote=True) |
450 | + _supersedes = Reference("id", "superseded_by_id", on_remote=True) |
451 | |
452 | @cachedproperty |
453 | def supersedes(self): |
454 | return self._supersedes |
455 | |
456 | - date_created = UtcDateTimeCol(notNull=True, default=DEFAULT) |
457 | - date_review_requested = UtcDateTimeCol(notNull=False, default=None) |
458 | - date_reviewed = UtcDateTimeCol(notNull=False, default=None) |
459 | + date_created = DateTime( |
460 | + allow_none=False, default=DEFAULT, tzinfo=timezone.utc |
461 | + ) |
462 | + date_review_requested = DateTime( |
463 | + allow_none=True, default=None, tzinfo=timezone.utc |
464 | + ) |
465 | + date_reviewed = DateTime( |
466 | + allow_none=True, default=None, tzinfo=timezone.utc |
467 | + ) |
468 | |
469 | @property |
470 | def target(self): |
471 | @@ -584,7 +626,7 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
472 | # although it has similar semantics. |
473 | return self.source_git_repository.namespace |
474 | |
475 | - root_message_id = StringCol(default=None) |
476 | + root_message_id = Unicode(default=None) |
477 | |
478 | @property |
479 | def title(self): |
480 | @@ -923,10 +965,10 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
481 | self._transitionToState( |
482 | BranchMergeProposalStatus.SUPERSEDED, registrant |
483 | ) |
484 | - # This sync update is needed as the add landing target does |
485 | - # a database query to identify if there are any active proposals |
486 | - # with the same source and target branches. |
487 | - self.syncUpdate() |
488 | + # This flush is needed as the add landing target does a database |
489 | + # query to identify if there are any active proposals with the same |
490 | + # source and target branches. |
491 | + Store.of(self).flush() |
492 | review_requests = list( |
493 | {(vote.reviewer, vote.review_type) for vote in self.votes} |
494 | ) |
495 | @@ -941,10 +983,10 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
496 | ) |
497 | if not break_link: |
498 | self.superseded_by = proposal |
499 | - # This sync update is needed to ensure that the transitive |
500 | - # properties of supersedes and superseded_by are visible to |
501 | - # the old and the new proposal. |
502 | - self.syncUpdate() |
503 | + # This flush is needed to ensure that the transitive properties of |
504 | + # supersedes and superseded_by are visible to the old and the new |
505 | + # proposal. |
506 | + Store.of(self).flush() |
507 | return proposal |
508 | |
509 | def _normalizeReviewType(self, review_type): |
510 | @@ -1068,7 +1110,7 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
511 | ): |
512 | job.destroySelf() |
513 | self._preview_diffs.remove() |
514 | - self.destroySelf() |
515 | + Store.of(self).remove(self) |
516 | |
517 | def getUnlandedSourceBranchRevisions(self): |
518 | """See `IBranchMergeProposal`.""" |
519 | @@ -1513,24 +1555,24 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
520 | person_ids = set() |
521 | for mp in branch_merge_proposals: |
522 | ids.add(mp.id) |
523 | - if mp.source_branchID is not None: |
524 | - source_branch_ids.add(mp.source_branchID) |
525 | - if mp.source_git_repositoryID is not None: |
526 | + if mp.source_branch_id is not None: |
527 | + source_branch_ids.add(mp.source_branch_id) |
528 | + if mp.source_git_repository_id is not None: |
529 | git_ref_keys.add( |
530 | - (mp.source_git_repositoryID, mp.source_git_path) |
531 | + (mp.source_git_repository_id, mp.source_git_path) |
532 | ) |
533 | git_ref_keys.add( |
534 | - (mp.target_git_repositoryID, mp.target_git_path) |
535 | + (mp.target_git_repository_id, mp.target_git_path) |
536 | ) |
537 | - if mp.prerequisite_git_repositoryID is not None: |
538 | + if mp.prerequisite_git_repository_id is not None: |
539 | git_ref_keys.add( |
540 | ( |
541 | - mp.prerequisite_git_repositoryID, |
542 | + mp.prerequisite_git_repository_id, |
543 | mp.prerequisite_git_path, |
544 | ) |
545 | ) |
546 | - person_ids.add(mp.registrantID) |
547 | - person_ids.add(mp.merge_reporterID) |
548 | + person_ids.add(mp.registrant_id) |
549 | + person_ids.add(mp.merge_reporter_id) |
550 | git_repository_ids = { |
551 | repository_id for repository_id, _ in git_ref_keys |
552 | } |
553 | @@ -1538,15 +1580,15 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
554 | branches = load_related( |
555 | Branch, |
556 | branch_merge_proposals, |
557 | - ("target_branchID", "prerequisite_branchID", "source_branchID"), |
558 | + ("target_branch_id", "prerequisite_branch_id", "source_branch_id"), |
559 | ) |
560 | repositories = load_related( |
561 | GitRepository, |
562 | branch_merge_proposals, |
563 | ( |
564 | - "target_git_repositoryID", |
565 | - "prerequisite_git_repositoryID", |
566 | - "source_git_repositoryID", |
567 | + "target_git_repository_id", |
568 | + "prerequisite_git_repository_id", |
569 | + "source_git_repository_id", |
570 | ), |
571 | ) |
572 | load(GitRef, git_ref_keys) |
573 | @@ -1579,9 +1621,9 @@ class BranchMergeProposal(SQLBase, BugLinkTargetMixin): |
574 | # Preload other merge proposals that supersede these. |
575 | supersedes_map = {} |
576 | for other_mp in load_referencing( |
577 | - BranchMergeProposal, branch_merge_proposals, ["superseded_byID"] |
578 | + BranchMergeProposal, branch_merge_proposals, ["superseded_by_id"] |
579 | ): |
580 | - supersedes_map[other_mp.superseded_byID] = other_mp |
581 | + supersedes_map[other_mp.superseded_by_id] = other_mp |
582 | for mp in branch_merge_proposals: |
583 | get_property_cache(mp).supersedes = supersedes_map.get(mp.id) |
584 | |
585 | @@ -1643,7 +1685,10 @@ class BranchMergeProposalGetter: |
586 | @staticmethod |
587 | def get(id): |
588 | """See `IBranchMergeProposalGetter`.""" |
589 | - return BranchMergeProposal.get(id) |
590 | + mp = IStore(BranchMergeProposal).get(BranchMergeProposal, id) |
591 | + if mp is None: |
592 | + raise NotFoundError(id) |
593 | + return mp |
594 | |
595 | @staticmethod |
596 | def getProposalsForContext(context, status=None, visible_by_user=None): |
597 | @@ -1666,7 +1711,7 @@ class BranchMergeProposalGetter: |
598 | """See `IBranchMergeProposalGetter`.""" |
599 | registrant_select = Select( |
600 | BranchMergeProposal.id, |
601 | - BranchMergeProposal.registrantID == participant.id, |
602 | + BranchMergeProposal.registrant_id == participant.id, |
603 | ) |
604 | |
605 | review_select = Select( |
606 | diff --git a/lib/lp/code/model/gitcollection.py b/lib/lp/code/model/gitcollection.py |
607 | index 1885d75..2481410 100644 |
608 | --- a/lib/lp/code/model/gitcollection.py |
609 | +++ b/lib/lp/code/model/gitcollection.py |
610 | @@ -367,7 +367,7 @@ class GenericGitCollection: |
611 | BranchMergeProposal, |
612 | And( |
613 | GitRepository.id |
614 | - == BranchMergeProposal.source_git_repositoryID, |
615 | + == BranchMergeProposal.source_git_repository_id, |
616 | *( |
617 | self._filter_expressions |
618 | + self._asymmetric_filter_expressions |
619 | @@ -376,7 +376,7 @@ class GenericGitCollection: |
620 | ), |
621 | Join( |
622 | Target, |
623 | - Target.id == BranchMergeProposal.target_git_repositoryID, |
624 | + Target.id == BranchMergeProposal.target_git_repository_id, |
625 | ), |
626 | ] |
627 | ) |
628 | @@ -451,7 +451,8 @@ class GenericGitCollection: |
629 | # Need to filter on GitRepository beyond the with constraints. |
630 | expressions += self._asymmetric_filter_expressions |
631 | expressions.append( |
632 | - BranchMergeProposal.source_git_repositoryID == GitRepository.id |
633 | + BranchMergeProposal.source_git_repository_id |
634 | + == GitRepository.id |
635 | ) |
636 | tables.append(GitRepository) |
637 | tables.extend(self._asymmetric_tables.values()) |
638 | @@ -507,14 +508,14 @@ class GenericGitCollection: |
639 | |
640 | expressions = [ |
641 | CodeReviewVoteReference.reviewer == reviewer, |
642 | - BranchMergeProposal.source_git_repositoryID.is_in( |
643 | + BranchMergeProposal.source_git_repository_id.is_in( |
644 | self._getRepositorySelect() |
645 | ), |
646 | ] |
647 | visibility = self._getRepositoryVisibilityExpression() |
648 | if visibility: |
649 | expressions.append( |
650 | - BranchMergeProposal.target_git_repositoryID.is_in( |
651 | + BranchMergeProposal.target_git_repository_id.is_in( |
652 | Select(GitRepository.id, visibility) |
653 | ) |
654 | ) |
655 | diff --git a/lib/lp/code/model/gitrepository.py b/lib/lp/code/model/gitrepository.py |
656 | index 37c5c08..8b3c3ea 100644 |
657 | --- a/lib/lp/code/model/gitrepository.py |
658 | +++ b/lib/lp/code/model/gitrepository.py |
659 | @@ -1417,7 +1417,7 @@ class GitRepository( |
660 | } |
661 | updated = set() |
662 | for kind in ("source", "target", "prerequisite"): |
663 | - repository_name = "%s_git_repositoryID" % kind |
664 | + repository_name = "%s_git_repository_id" % kind |
665 | path_name = "%s_git_path" % kind |
666 | commit_sha1_name = "%s_git_commit_sha1" % kind |
667 | old_column = partial(getattr, BranchMergeProposal) |
668 | @@ -1958,8 +1958,8 @@ class GitRepository( |
669 | seen_merge_proposal_ids.add(merge_proposal.id) |
670 | # Cannot use self.landing_candidates, because it ignores merged |
671 | # merge proposals. |
672 | - for merge_proposal in BranchMergeProposal.selectBy( |
673 | - target_git_repository=self |
674 | + for merge_proposal in Store.of(self).find( |
675 | + BranchMergeProposal, target_git_repository=self |
676 | ): |
677 | if merge_proposal.id not in seen_merge_proposal_ids: |
678 | deletion_operations.append( |
679 | @@ -1973,8 +1973,8 @@ class GitRepository( |
680 | ) |
681 | ) |
682 | seen_merge_proposal_ids.add(merge_proposal.id) |
683 | - for merge_proposal in BranchMergeProposal.selectBy( |
684 | - prerequisite_git_repository=self |
685 | + for merge_proposal in Store.of(self).find( |
686 | + BranchMergeProposal, prerequisite_git_repository=self |
687 | ): |
688 | if merge_proposal.id not in seen_merge_proposal_ids: |
689 | alteration_operations.append( |
690 | diff --git a/lib/lp/code/model/tests/test_branch.py b/lib/lp/code/model/tests/test_branch.py |
691 | index d31581b..6ea7f28 100644 |
692 | --- a/lib/lp/code/model/tests/test_branch.py |
693 | +++ b/lib/lp/code/model/tests/test_branch.py |
694 | @@ -60,6 +60,7 @@ from lp.code.interfaces.branchlookup import IBranchLookup |
695 | from lp.code.interfaces.branchmergeproposal import ( |
696 | BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES, |
697 | ) |
698 | +from lp.code.interfaces.branchmergeproposal import IBranchMergeProposalGetter |
699 | from lp.code.interfaces.branchnamespace import ( |
700 | IBranchNamespacePolicy, |
701 | IBranchNamespaceSet, |
702 | @@ -87,7 +88,6 @@ from lp.code.model.branchjob import ( |
703 | BranchScanJob, |
704 | ReclaimBranchSpaceJob, |
705 | ) |
706 | -from lp.code.model.branchmergeproposal import BranchMergeProposal |
707 | from lp.code.model.branchrevision import BranchRevision |
708 | from lp.code.model.codereviewcomment import CodeReviewComment |
709 | from lp.code.model.revision import Revision |
710 | @@ -110,7 +110,6 @@ from lp.registry.tests.test_accesspolicy import get_policies_for_artifact |
711 | from lp.services.config import config |
712 | from lp.services.database.constants import UTC_NOW |
713 | from lp.services.database.interfaces import IStore |
714 | -from lp.services.database.sqlobject import SQLObjectNotFound |
715 | from lp.services.features.testing import FeatureFixture |
716 | from lp.services.job.interfaces.job import JobStatus |
717 | from lp.services.job.runner import JobRunner |
718 | @@ -1701,20 +1700,24 @@ class TestBranchDeletionConsequences(TestCase): |
719 | """Merge proposal source branches can be deleted with break_links.""" |
720 | merge_proposal1, merge_proposal2 = self.makeMergeProposals() |
721 | merge_proposal1_id = merge_proposal1.id |
722 | - BranchMergeProposal.get(merge_proposal1_id) |
723 | + getUtility(IBranchMergeProposalGetter).get(merge_proposal1_id) |
724 | self.branch.destroySelf(break_references=True) |
725 | self.assertRaises( |
726 | - SQLObjectNotFound, BranchMergeProposal.get, merge_proposal1_id |
727 | + NotFoundError, |
728 | + getUtility(IBranchMergeProposalGetter).get, |
729 | + merge_proposal1_id, |
730 | ) |
731 | |
732 | def test_deleteMergeProposalTarget(self): |
733 | """Merge proposal target branches can be deleted with break_links.""" |
734 | merge_proposal1, merge_proposal2 = self.makeMergeProposals() |
735 | merge_proposal1_id = merge_proposal1.id |
736 | - BranchMergeProposal.get(merge_proposal1_id) |
737 | + getUtility(IBranchMergeProposalGetter).get(merge_proposal1_id) |
738 | merge_proposal1.target_branch.destroySelf(break_references=True) |
739 | self.assertRaises( |
740 | - SQLObjectNotFound, BranchMergeProposal.get, merge_proposal1_id |
741 | + NotFoundError, |
742 | + getUtility(IBranchMergeProposalGetter).get, |
743 | + merge_proposal1_id, |
744 | ) |
745 | |
746 | def test_deleteMergeProposalDependent(self): |
747 | diff --git a/lib/lp/code/model/tests/test_gitrepository.py b/lib/lp/code/model/tests/test_gitrepository.py |
748 | index f74776f..057c80a 100644 |
749 | --- a/lib/lp/code/model/tests/test_gitrepository.py |
750 | +++ b/lib/lp/code/model/tests/test_gitrepository.py |
751 | @@ -76,6 +76,7 @@ from lp.code.event.git import GitRefsUpdatedEvent |
752 | from lp.code.interfaces.branchmergeproposal import ( |
753 | BRANCH_MERGE_PROPOSAL_FINAL_STATES as FINAL_STATES, |
754 | ) |
755 | +from lp.code.interfaces.branchmergeproposal import IBranchMergeProposalGetter |
756 | from lp.code.interfaces.cibuild import ( |
757 | CI_WEBHOOKS_FEATURE_FLAG, |
758 | ICIBuild, |
759 | @@ -103,7 +104,6 @@ from lp.code.interfaces.revisionstatus import ( |
760 | IRevisionStatusArtifactSet, |
761 | IRevisionStatusReportSet, |
762 | ) |
763 | -from lp.code.model.branchmergeproposal import BranchMergeProposal |
764 | from lp.code.model.branchmergeproposaljob import ( |
765 | BranchMergeProposalJob, |
766 | BranchMergeProposalJobType, |
767 | @@ -156,7 +156,6 @@ from lp.services.config import config |
768 | from lp.services.database.constants import UTC_NOW |
769 | from lp.services.database.interfaces import IStore |
770 | from lp.services.database.sqlbase import get_transaction_timestamp |
771 | -from lp.services.database.sqlobject import SQLObjectNotFound |
772 | from lp.services.features.testing import FeatureFixture |
773 | from lp.services.identity.interfaces.account import AccountStatus |
774 | from lp.services.job.interfaces.job import JobStatus |
775 | @@ -1553,10 +1552,12 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory): |
776 | # break_references. |
777 | merge_proposal1, merge_proposal2 = self.makeMergeProposals() |
778 | merge_proposal1_id = merge_proposal1.id |
779 | - BranchMergeProposal.get(merge_proposal1_id) |
780 | + getUtility(IBranchMergeProposalGetter).get(merge_proposal1_id) |
781 | self.repository.destroySelf(break_references=True) |
782 | self.assertRaises( |
783 | - SQLObjectNotFound, BranchMergeProposal.get, merge_proposal1_id |
784 | + NotFoundError, |
785 | + getUtility(IBranchMergeProposalGetter).get, |
786 | + merge_proposal1_id, |
787 | ) |
788 | |
789 | def test_delete_merge_proposal_target(self): |
790 | @@ -1564,12 +1565,14 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory): |
791 | # break_references. |
792 | merge_proposal1, merge_proposal2 = self.makeMergeProposals() |
793 | merge_proposal1_id = merge_proposal1.id |
794 | - BranchMergeProposal.get(merge_proposal1_id) |
795 | + getUtility(IBranchMergeProposalGetter).get(merge_proposal1_id) |
796 | merge_proposal1.target_git_repository.destroySelf( |
797 | break_references=True |
798 | ) |
799 | self.assertRaises( |
800 | - SQLObjectNotFound, BranchMergeProposal.get, merge_proposal1_id |
801 | + NotFoundError, |
802 | + getUtility(IBranchMergeProposalGetter).get, |
803 | + merge_proposal1_id, |
804 | ) |
805 | |
806 | def test_delete_merge_proposal_prerequisite(self): |
807 | @@ -1728,7 +1731,9 @@ class TestGitRepositoryDeletionConsequences(TestCaseWithFactory): |
808 | merge_proposal, "blah", merge_proposal.deleteProposal |
809 | )() |
810 | self.assertRaises( |
811 | - SQLObjectNotFound, BranchMergeProposal.get, merge_proposal_id |
812 | + NotFoundError, |
813 | + getUtility(IBranchMergeProposalGetter).get, |
814 | + merge_proposal_id, |
815 | ) |
816 | |
817 | def test_DeleteCodeImport(self): |
LGTM 👍