[Patchew-devel] [PATCH 6/9] remove obsoleted-by property

Paolo Bonzini pbonzini at redhat.com
Thu Jan 16 15:09:06 UTC 2020


Move the obsoleted-by property to the Topic model, which
ensures it is consistent for all series in the same topic.

While at it, fix the obsoletion check to use the date
if the versions are equal.  This works better if the
submitter forgets to bump the version.

Signed-off-by: Paolo Bonzini <pbonzini at redhat.com>
---
 api/migrations/0059_populate_topic_latest.py |  32 ++++++++++
 api/models.py                                |   3 +
 mods/tags.py                                 |  62 +++++++++----------
 tests/data/0030-obsolete-by-v3.mbox.gz       | Bin 0 -> 2302 bytes
 tests/test_tags.py                           |  40 +++++++++---
 5 files changed, 97 insertions(+), 40 deletions(-)
 create mode 100644 api/migrations/0059_populate_topic_latest.py
 create mode 100644 tests/data/0030-obsolete-by-v3.mbox.gz

diff --git a/api/migrations/0059_populate_topic_latest.py b/api/migrations/0059_populate_topic_latest.py
new file mode 100644
index 0000000..eae7607
--- /dev/null
+++ b/api/migrations/0059_populate_topic_latest.py
@@ -0,0 +1,32 @@
+# -*- coding: utf-8 -*-
+from __future__ import unicode_literals
+
+from django.conf import settings
+from django.db import migrations
+
+
+def topic_fill_latest(apps, schema_editor):
+    Message = apps.get_model("api", "Message")
+    Topic = apps.get_model("api", "Topic")
+    series = Message.objects.filter(topic__isnull=False)
+    for m in series.filter(is_obsolete=False):
+        m.topic.latest = m
+    for m in series.filter(is_obsolete=True):
+        if "obsoleted-by" in m.properties:
+            del m.properties["obsoleted-by"]
+            m.save()
+
+
+def message_set_obsoleted_by(apps, schema_editor):
+    Message = apps.get_model("api", "Message")
+    for m in Message.objects.filter(is_obsolete=True, topic__latest__isnull=False):
+        m.properties["obsoleted-by"] = m.topic.latest.message_id
+        m.save()
+
+
+class Migration(migrations.Migration):
+    dependencies = [("api", "0058_topic_latest")]
+
+    operations = [
+        migrations.RunPython(topic_fill_latest, reverse_code=message_set_obsoleted_by)
+    ]
diff --git a/api/models.py b/api/models.py
index f5a1114..ab122e6 100644
--- a/api/models.py
+++ b/api/models.py
@@ -546,6 +546,9 @@ class TopicManager(models.Manager):
 
 class Topic(models.Model):
     objects = TopicManager()
+    latest = models.ForeignKey(
+        "Message", on_delete=models.SET_NULL, null=True, related_name="+"
+    )
 
 
 class Message(models.Model):
diff --git a/mods/tags.py b/mods/tags.py
index 55cc9a6..ec2cc49 100644
--- a/mods/tags.py
+++ b/mods/tags.py
@@ -79,17 +79,11 @@ series cover letter, patch mail body and their replies.
             return
 
         def newer_than(m1, m2):
-            return m1.version > m2.version and m1.date >= m2.date
-
-        for m in series.get_alternative_revisions():
-            if newer_than(m, series):
-                series.is_obsolete = True
-                series.save()
-                series.set_property("obsoleted-by", m.message_id)
-            elif newer_than(series, m):
-                m.is_obsolete = True
-                m.save()
-                m.set_property("obsoleted-by", series.message_id)
+            if m1.version > m2.version:
+                return m1.date >= m2.date
+            if m1.version < m2.version:
+                return False
+            return m1.date > m2.date
 
         updated = self.update_tags(series)
 
@@ -127,6 +121,14 @@ series cover letter, patch mail body and their replies.
         if updated:
             emit_event("TagsUpdate", series=series)
 
+        if not series.topic.latest or newer_than(series, series.topic.latest):
+            series.topic.latest = series
+            series.topic.save()
+        for m in series.get_alternative_revisions():
+            if not m.is_obsolete and m.topic.latest != m:
+                m.is_obsolete = True
+                m.save()
+
     def parse_message_tags(self, m):
         r = []
         for l in m.get_body().splitlines():
@@ -156,29 +158,25 @@ series cover letter, patch mail body and their replies.
                     "char": "R",
                 }
             )
-        ob = message.get_property("obsoleted-by")
-        if ob:
-            new = Message.objects.find_series(ob, message.project.name)
-            if new is not None:
-                message.status_tags.append(
-                    {
-                        "title": "Has a newer version: " + new.subject,
-                        "type": "default",
-                        "char": "O",
-                        "row_class": "obsolete",
-                    }
-                )
+        if message.is_obsolete:
+            message.status_tags.append(
+                {
+                    "title": "Has a newer version: " + message.topic.latest.subject,
+                    "type": "default",
+                    "char": "O",
+                    "row_class": "obsolete",
+                }
+            )
 
     def get_obsoleted_by(self, message, request, format):
-        obsoleted_by = message.get_property("obsoleted-by")
-        if not obsoleted_by:
-            return None
-        return rest_framework.reverse.reverse(
-            "series-detail",
-            kwargs={"projects_pk": message.project.id, "message_id": obsoleted_by},
-            request=request,
-            format=format,
-        )
+        if message.is_obsolete:
+            obsoleted_by = message.topic.latest.message_id
+            return rest_framework.reverse.reverse(
+                "series-detail",
+                kwargs={"projects_pk": message.project.id, "message_id": obsoleted_by},
+                request=request,
+                format=format,
+            )
 
     def get_reviewers(self, message, request, format):
         reviewers = message.get_property("reviewers", [])
diff --git a/tests/data/0030-obsolete-by-v3.mbox.gz b/tests/data/0030-obsolete-by-v3.mbox.gz
new file mode 100644
index 0000000000000000000000000000000000000000..abd55385f95f2be5d7ad487d23301c45729eb371
GIT binary patch
literal 2302
zcmZw1X*?4O003|~GRK;dn;elgXU!2YVsbRC<&JVRnMa1qkPtOeScNA~2(!YJYK|dC
zX(nYz%sI2PkYi}>-skuGz5j>5`h5U^8>ewaNW|C}YHWZFx`7Q3jtMphiZ_Ub8eR{=
z#tHrM4s)A1CBOS1xohGDg at U%t_P?iQzQzaai;Y7&ZPmG=z;w?;rrF=?!<`bY?QLIS
zX at UT56Kj9}g19+^fAXRUj$Bq_ke at C~s2(>^etol$nX=X>6 at F+{o5V<3Ang^vo?9t_
zhrg0Bvcw4TFFU%dFvNeepc+=vD6 at QzuEn<Bu+vqN;8_D1DIpJ$T}dCxQMnA1Mrmr6
zmJvx<`ZEeQfdam%oimFQ4k|==7Y+!CW(gPC%C&=(vMzQ837pe)_jMWVUxN$}k!D(P
z^X9fk5!v)y>4Pn;KQ<$5I1LS{Q<4B|z=K_Z-LZ?$VD*8Pq51HN8PZjS*tX=a4N|s#
z#M>RC8y9?%TYQjn^^$2yh{`HancGr289m=b&e55zJRGfEm`Nc}Smy`78N82HCd`=t
z$D#_?uU&pKk7cJio(>pUF44EUv%a5~?Tc-s9e}?v1wx*BzTDDxS-d%2aQ`sGytaCZ
zR>_JiRFSEI_$qP|<l__F5d(@4x|`X)b5xv3O^s`RHS+ at s-IM0_?QKS7Xx8e1CYoD4
z!HU%gGpoSzYVQJ06}%&bB4x?t{XGlpM9jeWSHFq3ezMP)5fik~k<lD&*xE6FL6lEU
ztNRz;7^L at cbawE?kDGUPVlQh`KVT^C{@pHb%tMCCL}nfC^b{D+QMFe~9Tr?OmRpZ^
zQh)c1_ at 0a<hdeyD?NMiLMu$C_n;Y<l>AHHD13mDFp~#5KpCU%jo>1$Pz4<v@|F7Oq
zE*NLTV38F#_5L;51e3dUVUS5~nEClQ4fU^aR;Hd8J2T#~<Rh4X7iSy^jd at MzREMLy
zr}(A17f!5 at 6-A$|jpw)oL~XM;m)#ilRfWstrN$sZ2zsg{Y<O9HzrLJ@#TT6tH at yuL
zXBKhOi9uu<Dam!WiM1ffWoe=((1U+X3*yc=r&H`*-;5Q`w4mw}lpIEibiyy~`v*7%
zFy!|&PxeFiR)kGuO%6iqW{nlMPxQ~j&0S6T*TOVh(8 at p$$CzGK73qz!D2m3?r790S
z!)m4PL&TxxgyV1|w5D>l#ZPKI-r%il(-^f?B2n3`s5Hc(vRm!ej4<j_MvGKmV3-Ri
z|IX(#AO&{-7S2$ztJed*B}`*z!M^pGdLO>sJ6~+E>qP5P)=h^h7=AI#_Ko2MCK5&?
zQygE_7{0Z3<{=IhNlgUJ*vz-TFCe*_4Frg56?68~5}?{ZpCkJ4EuHds^B_i at vbP^{
zZnlnKI)oYbY- at tUwA{h#{m_oCf8xNNuntLQu-z_tr8zQsvo`iiS$JlX(p+xA+~J6`
zCS?+<*r=K=3sON9?jnlDVnb=s)3bsu)rn5mAEo%11S$WM#pscr5Xq1fr7r!74CyRe
zbksWuG2S$B;CZ^<!*?eA>I|`OT+w@**7=q*2MN!#e$xh=jcF;IeBP|@gJ{E*UUGs(
zE-j&_8#w8pr2?=C6j^9EeSXlpk$S1l(I6}Aac5W-9c(E_{Z7Q<b19cMEx#}JELxFo
z=Z*X^kWBggn)E4NM*N=5*?=GW!ud;b^3*6x39q8zSrI4Cj>*2xk>va(FmT2-KUu-}
zNFXO(uqa5;@qdNsP6klqYO)g*lF9{fihJ$vH5VqW*}=HF#r89&Yz=24Q&UcWVpqwx
z%=mV*a>sf+7x5`{YO`GO>Bwo<>B84*dS#Eh=aqq~x)MlHA95+yIt?tQ)VTa9I&_Cm
zD_?QsOMvR54-%`>ow7G6i27Yz=k}#Pj;*jtK6>5Az^`$!ep|Vnob(=JmPJEh?HUH>
z=W`6QDl7YZOUJsVBAN#u*{H<{I$jYyr_^TuM*cFhfWgzst0Pp{Rk#BqGu+XI`CB44
zI-sN>-tgxpJil1yiN?g&bI{udAuiZnQ7g6=%CfS1$jLfpZZHveQxg$K)Rd~rAQ{88
z(a=i?-AvUsif9Ql=D~v(%nKJbwS+;Usdt~EuYbb6K0_QA$XY&HD|TPL`bo;ZF72xD
zBsXH!#Vr+{lrQP=5k7stq?eN`pcg9&euy3yy1Kq&@;)2-6EGM6V<&FlPiw at h+AMH=
z2G8X=3cIrIR0&-_Lf8rXnUy#z!td+p9}3=HsF-?3>ufw#%r3LCw+i71JtS&6?n&Fv
zaeh>cYZ2^E7VPdGK>$#ax>Tv9>;oqx^FT1E4O!`tD=m7wE9;tnwq^A-;*la~&~2Y-
zd5FMX?b#vz#)tBrSe*D=G`_-u;pyao5;ju}nmTy?q1PA}|LTQDpm&AaEB%)dKY<?W
zYNs(6+;}yW5X{gv%c%^<V%v;4doY<B;tgJ#X&^D?MrLooO;AX#jUvRTwmZaLeb|$R
zS$>a@@8<ZxujqeF?6QqF(*kmRixkFFcRO}qa5qp`&|X18+VV=$^wq%&+Aqu3@%JU;
zlSj9_^9X{OfXs4+R0l3w-8KiQsU8@@k6{NDHoa~QbhkRs)@)8rU|LgZm27}UzYo>K
z`=NQ^GLvF4#H>tTzmwso^cH26XX--j1)M1MCSLL6Xl>AZ*TUT*o1)8wXODPstcl|C
zPolT at dMk?bD=IEn)VDNU;A$$=^;Uq2cO(s}%4}!3N8UHArq*L}gDdB0P32*)o=%yd
zsh=6_3(5`A5O4vRCw_A|UAU)w$eKPQ=$U_qZp;j)Krl|+M`-)!J6DeaJ(6r_dss3)
z59#-Zy;G6nZ at V$=MuGj#OQ!li!|Z2muE^+u5sTlY!6N)6mKJP0q5wHPJQ|&%?R9VR
zFfbV$KW6!4FHA?witln%^w44Z>s5tiTy^P4Z%7h}<MrQky%_l8Ngw+t8y0_FW~z^?
zka%91N&lCP{+3+T6zauRUX}lUD+x1gb6kfepB(Dj3w5`7<G$-M^zwgNO802{voEfJ
K%bYkZB=j#ysD%0e

literal 0
HcmV?d00001

diff --git a/tests/test_tags.py b/tests/test_tags.py
index 34a682d..38ded37 100755
--- a/tests/test_tags.py
+++ b/tests/test_tags.py
@@ -12,6 +12,7 @@ import email
 import email.parser
 import email.policy
 from mbox import decode_payload
+from api.models import Message
 
 from .patchewtest import PatchewTestCase, main
 
@@ -90,18 +91,41 @@ class ImportTest(PatchewTestCase):
         self.assertIn("Reviewed-By: Fam Zheng <famz at redhat.com>", resp.data["tags"])
         self.assertIn("tESTed-bY: Fam Zheng <famz at redhat.com>", resp.data["tags"])
 
-    def test_obsoleted_by(self):
+    def do_test_obsoleted_by(self, old_id, new_id):
+        m1 = Message.objects.find_series(old_id, self.p.name)
+        m2 = Message.objects.find_series(new_id, self.p.name)
+        self.assertEqual(m1.topic, m2.topic)
+
+        resp = self.api_client.get(self.PROJECT_BASE + "series/" + old_id + "/")
+        self.assertEqual(
+            self.PROJECT_BASE + "series/" + new_id + "/", resp.data["obsoleted_by"]
+        )
+        resp = self.api_client.get(self.PROJECT_BASE + "series/" + new_id + "/")
+        self.assertEqual(None, resp.data["obsoleted_by"])
+
+    def test_obsoleted_by_date_out_of_order(self):
         self.cli_login()
         self.cli_import("0009-obsolete-by.mbox.gz")
         self.cli_logout()
-        OLD_ID = "20160628014747.20971-1-famz at redhat.com"
-        NEW_ID = "20160628014747.20971-2-famz at redhat.com"
-        resp = self.api_client.get(self.PROJECT_BASE + "series/" + OLD_ID + "/")
-        self.assertEqual(
-            self.PROJECT_BASE + "series/" + NEW_ID + "/", resp.data["obsoleted_by"]
+        # note that the "v3" actually has an older date than v2 in the testcase
+        self.do_test_obsoleted_by(
+            "20160628014747.20971-1-famz at redhat.com",
+            "20160628014747.20971-2-famz at redhat.com",
+        )
+
+    def test_obsoleted_by_v3(self):
+        self.cli_login()
+        self.cli_import("0030-obsolete-by-v3.mbox.gz")
+        self.cli_logout()
+        # note that the "v3" actually has an older date than v2 in the testcase
+        self.do_test_obsoleted_by(
+            "20160628014747.20971-2-famz at redhat.com",
+            "20160628014747.20971-3-famz at redhat.com",
+        )
+        self.do_test_obsoleted_by(
+            "20160628014747.20971-1-famz at redhat.com",
+            "20160628014747.20971-3-famz at redhat.com",
         )
-        resp = self.api_client.get(self.PROJECT_BASE + "series/" + NEW_ID + "/")
-        self.assertEqual(None, resp.data["obsoleted_by"])
 
 
 if __name__ == "__main__":
-- 
2.21.0





More information about the Patchew-devel mailing list