Merge ~pappacena/launchpad:comment-editing-model into launchpad:master

[Thiago F. Pappacena]

Mapping database initial structure for message editing

More changes will be added in a future MP in order to adjust BugComment, CodeReviewComment and QuestionMessage to use the new methods. These changes were splitted to make the review process easier.

[Colin Watson (cjwatson)]

review: Approve

[Thiago F. Pappacena (pappacena)]

Addressed all the comments.

[Colin Watson (cjwatson)]

review: Approve

[Thiago F. Pappacena (pappacena)]

Pushed the requested changes. This should be good to go now.

diff --git a/database/schema/security.cfg b/database/schema/security.cfg
index b13d4a8..8225ec9 100644
--- a/database/schema/security.cfg
+++ b/database/schema/security.cfg
@@ -232,7 +232,9 @@ public.logintoken                       = SELECT, INSERT, UPDATE, DELETE
 public.mailinglist                      = SELECT, INSERT, UPDATE, DELETE
 public.mailinglistsubscription          = SELECT, INSERT, UPDATE, DELETE
 public.messageapproval                  = SELECT, INSERT, UPDATE, DELETE
-public.messagechunk                     = SELECT, INSERT
+public.messagechunk                     = SELECT, INSERT, DELETE
+public.messagerevision                  = SELECT, INSERT, UPDATE, DELETE
+public.messagerevisionchunk             = SELECT, INSERT, DELETE
 public.milestone                        = SELECT, INSERT, UPDATE, DELETE
 public.milestonetag                     = SELECT, INSERT, UPDATE, DELETE
 public.mirrorcdimagedistroseries        = SELECT, INSERT, DELETE
diff --git a/lib/lp/bugs/browser/tests/test_bugcomment.py b/lib/lp/bugs/browser/tests/test_bugcomment.py
index c1877a5..e5baac1 100644
--- a/lib/lp/bugs/browser/tests/test_bugcomment.py
+++ b/lib/lp/bugs/browser/tests/test_bugcomment.py
@@ -1,4 +1,4 @@
-# Copyright 2010-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2010-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Tests for the bugcomment module."""
@@ -35,6 +35,7 @@ from lp.testing import (
     BrowserTestCase,
     celebrity_logged_in,
     login_person,
+    person_logged_in,
     TestCase,
     TestCaseWithFactory,
     verifyObject,
@@ -300,7 +301,10 @@ class TestBugCommentImplementsInterface(TestCaseWithFactory):
         bug_message = self.factory.makeBugComment()
         bugtask = bug_message.bugs[0].bugtasks[0]
         bug_comment = BugComment(1, bug_message, bugtask)
-        verifyObject(IBugComment, bug_comment)
+        # Runs verifyObject logged in as the bug owner, so we don't fail on
+        # attributes that are not public to everyone.
+        with person_logged_in(bug_message.owner):
+            verifyObject(IBugComment, bug_comment)
 
     def test_download_url(self):
         """download_url is provided and works as expected."""
diff --git a/lib/lp/security.py b/lib/lp/security.py
index 412f497..1eeca4e 100644
--- a/lib/lp/security.py
+++ b/lib/lp/security.py
@@ -187,6 +187,7 @@ from lp.services.identity.interfaces.account import IAccount
 from lp.services.identity.interfaces.emailaddress import IEmailAddress
 from lp.services.librarian.interfaces import ILibraryFileAliasWithParent
 from lp.services.messages.interfaces.message import IMessage
+from lp.services.messages.interfaces.messagerevision import IMessageRevision
 from lp.services.oauth.interfaces import (
     IOAuthAccessToken,
     IOAuthRequestToken,
@@ -3182,6 +3183,24 @@ class SetMessageVisibility(AuthorizationBase):
         return (user.in_admin or user.in_registry_experts)
 
 
+class EditMessage(AuthorizationBase):
+    permission = 'launchpad.Edit'
+    usedfor = IMessage
+
+    def checkAuthenticated(self, user):
+        """Only message owner can edit it."""
+        return user.isOwner(self.obj)
+
+
+class EditMessageRevision(DelegatedAuthorization):
+    permission = 'launchpad.Edit'
+    usedfor = IMessageRevision
+
+    def __init__(self, obj):
+        super(EditMessageRevision, self).__init__(
+            obj, obj.message, 'launchpad.Edit')
+
+
 class ViewPublisherConfig(AdminByAdminsTeam):
     usedfor = IPublisherConfig
 
diff --git a/lib/lp/services/messages/configure.zcml b/lib/lp/services/messages/configure.zcml
index e989fe2..19cb3c3 100644
--- a/lib/lp/services/messages/configure.zcml
+++ b/lib/lp/services/messages/configure.zcml
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
+<!-- Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
      GNU Affero General Public License version 3 (see the file LICENSE).
 -->
 
@@ -10,9 +10,9 @@
     i18n_domain="launchpad">
 
     <!-- Message -->
-    <class
-        class="lp.services.messages.model.message.Message">
-        <allow interface="lp.services.messages.interfaces.message.IMessage" />
+    <class class="lp.services.messages.model.message.Message">
+        <allow
+            interface="lp.services.messages.interfaces.message.IMessageView" />
         <!-- setVisible is required to allow IBug.setCommentVisibility() to
              change the visibility attribute whilst still ensuring restricted
              access to the attribute via the API.-->
@@ -22,6 +22,26 @@
         <require
             permission="launchpad.Admin"
             set_attributes="visible"/>
+        <require
+            permission="launchpad.Edit"
+            interface="lp.services.messages.interfaces.message.IMessageEdit" />
+    </class>
+
+    <!-- MessageRevision -->
+    <class
+        class="lp.services.messages.model.messagerevision.MessageRevision">
+        <allow
+            interface="lp.services.messages.interfaces.messagerevision.IMessageRevisionView"/>
+        <require
+            permission="launchpad.Edit"
+            interface="lp.services.messages.interfaces.messagerevision.IMessageRevisionEdit"/>
+    </class>
+
+    <!-- MessageChunk -->
+    <class
+        class="lp.services.messages.model.messagerevision.MessageRevisionChunk">
+        <allow
+            interface="lp.services.messages.interfaces.messagerevision.IMessageRevisionChunk"/>
     </class>
 
     <class class="lp.services.messages.interfaces.message.IndexedMessage">
diff --git a/lib/lp/services/messages/interfaces/message.py b/lib/lp/services/messages/interfaces/message.py
index 7e18e7d..70d50b7 100644
--- a/lib/lp/services/messages/interfaces/message.py
+++ b/lib/lp/services/messages/interfaces/message.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -49,9 +49,19 @@ from lp.services.librarian.interfaces import ILibraryFileAlias
 from lp.services.webservice.apihelpers import patch_reference_property
 
 
-@exported_as_webservice_entry('message')
-class IMessage(Interface):
-    """A message.
+class IMessageEdit(Interface):
+
+    def editContent(new_content):
+        """Edit the content of this message, generating a new message
+        revision with the old content.
+        """
+
+    def deleteContent():
+        """Deletes this message content."""
+
+
+class IMessageView(Interface):
+    """Public attributes for message.
 
     This is like an email (RFC822) message, though it could be created through
     the web as well.
@@ -61,6 +71,15 @@ class IMessage(Interface):
     datecreated = exported(
         Datetime(title=_('Date Created'), required=True, readonly=True),
         exported_as='date_created')
+
+    date_last_edited = Datetime(
+        title=_('When this message was last edited'), required=False,
+        readonly=True)
+
+    date_deleted = Datetime(
+        title=_('When this message was deleted'), required=False,
+        readonly=True)
+
     subject = exported(
         TextLine(title=_('Subject'), required=True, readonly=True))
 
@@ -87,6 +106,8 @@ class IMessage(Interface):
 
     chunks = Attribute(_('Message pieces'))
 
+    revisions = Attribute(_('Message revision history'))
+
     text_contents = exported(
         Text(title=_('All the text/plain chunks joined together as a '
                      'unicode string.')),
@@ -115,6 +136,11 @@ class IMessage(Interface):
         """Return None because messages are not threaded over the API."""
 
 
+@exported_as_webservice_entry('message')
+class IMessage(IMessageEdit, IMessageView):
+    """A Message."""
+
+
 # Fix for self-referential schema.
 patch_reference_property(IMessage, 'parent', IMessage)
 
diff --git a/lib/lp/services/messages/interfaces/messagerevision.py b/lib/lp/services/messages/interfaces/messagerevision.py
 new file mode 100644
index 0000000..49d3b9f
--- /dev/null
+++ b/lib/lp/services/messages/interfaces/messagerevision.py
@@ -0,0 +1,69 @@
+# Copyright 2019-2021 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Message revision history."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__all__ = [
+    'IMessageRevision',
+    'IMessageRevisionChunk',
+    ]
+
+from lazr.restful.fields import Reference
+from zope.interface import (
+    Attribute,
+    Interface,
+    )
+from zope.schema import (
+    Datetime,
+    Int,
+    Text,
+    )
+
+from lp import _
+from lp.services.messages.interfaces.message import IMessage
+
+
+class IMessageRevisionView(Interface):
+    """IMessageRevision readable attributes."""
+    id = Int(title=_("ID"), required=True, readonly=True)
+
+    revision = Int(title=_("Revision number"), required=True, readonly=True)
+
+    content = Text(
+        title=_("The message at the given revision"),
+        required=True, readonly=True)
+
+    message = Reference(
+        title=_('The current message of this revision.'),
+        schema=IMessage, required=True, readonly=True)
+
+    date_created = Datetime(
+        title=_("The time when this message revision was created."),
+        required=True, readonly=True)
+
+    date_deleted = Datetime(
+        title=_("The time when this message revision was created."),
+        required=False, readonly=True)
+
+    chunks = Attribute(_('Message pieces'))
+
+
+class IMessageRevisionEdit(Interface):
+    """IMessageRevision editable attributes."""
+
+    def deleteContent():
+        """Deletes this MessageRevision content."""
+
+
+class IMessageRevision(IMessageRevisionView, IMessageRevisionEdit):
+    """A historical revision of a IMessage."""
+
+
+class IMessageRevisionChunk(Interface):
+    id = Int(title=_('ID'), required=True, readonly=True)
+    messagerevision = Int(
+        title=_('MessageRevision'), required=True, readonly=True)
+    sequence = Int(title=_('Sequence order'), required=True, readonly=True)
+    content = Text(title=_('Text content'), required=True, readonly=True)
diff --git a/lib/lp/services/messages/model/message.py b/lib/lp/services/messages/model/message.py
index e592daa..be685d6 100644
--- a/lib/lp/services/messages/model/message.py
+++ b/lib/lp/services/messages/model/message.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2020 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -40,7 +40,9 @@ from sqlobject import (
 from storm.locals import (
     And,
     DateTime,
+    Desc,
     Int,
+    Max,
     Reference,
     Store,
     Storm,
@@ -71,7 +73,14 @@ from lp.services.messages.interfaces.message import (
     IUserToUserEmail,
     UnknownSender,
     )
-from lp.services.propertycache import cachedproperty
+from lp.services.messages.model.messagerevision import (
+    MessageRevision,
+    MessageRevisionChunk,
+    )
+from lp.services.propertycache import (
+    cachedproperty,
+    get_property_cache,
+    )
 
 
 def utcdatetime_from_field(field_value):
@@ -100,6 +109,8 @@ class Message(SQLBase):
     _table = 'Message'
     _defaultOrder = '-id'
     datecreated = UtcDateTimeCol(notNull=True, default=UTC_NOW)
+    date_deleted = UtcDateTimeCol(notNull=False, default=None)
+    date_last_edited = UtcDateTimeCol(notNull=False, default=None)
     subject = StringCol(notNull=False, default=None)
     owner = ForeignKey(
         dbName='owner', foreignKey='Person',
@@ -164,6 +175,82 @@ class Message(SQLBase):
         """See `IMessage`."""
         return None
 
+    @cachedproperty
+    def revisions(self):
+        """See `IMessage`."""
+        return list(Store.of(self).find(
+            MessageRevision,
+            MessageRevision.message == self
+        ).order_by(Desc(MessageRevision.revision)))
+
+    def editContent(self, new_content):
+        """See `IMessage`."""
+        store = Store.of(self)
+
+        # Move the old content to a new revision.
+        date_created = (
+            self.date_last_edited if self.date_last_edited is not None
+            else self.datecreated)
+        current_rev_num = store.find(
+            Max(MessageRevision.revision),
+            MessageRevision.message == self).one()
+        rev_num = (current_rev_num or 0) + 1
+        rev = MessageRevision(
+            message=self, revision=rev_num, date_created=date_created)
+        self.date_last_edited = UTC_NOW
+        store.add(rev)
+
+        # Move the current text content to the recently created revision.
+        used_seq_numbers = set()
+        for chunk in self._chunks:
+            if chunk.blob is None:
+                revision_chunk = MessageRevisionChunk(
+                    rev, chunk.sequence, chunk.content)
+                store.add(revision_chunk)
+                store.remove(chunk)
+            else:
+                used_seq_numbers.add(chunk.sequence)
+
+        # Spot sequence number gaps.
+        # If there is a gap in sequence numbers, use it. Otherwise, use the
+        # max sequence number + 1.
+        min_gap = None
+        for i in range(1, len(used_seq_numbers) + 1):
+            if i not in used_seq_numbers:
+                min_gap = i
+                break
+        if min_gap is None:
+            new_seq = max(used_seq_numbers) + 1 if len(used_seq_numbers) else 1
+        else:
+            new_seq = min_gap
+
+        # Create the new content.
+        new_chunk = MessageChunk(
+            message=self, sequence=new_seq, content=new_content)
+        store.add(new_chunk)
+
+        store.flush()
+
+        # Clean up caches.
+        del get_property_cache(self).text_contents
+        del get_property_cache(self).chunks
+        del get_property_cache(self).revisions
+        return rev
+
+    def deleteContent(self):
+        """See `IMessage`."""
+        store = Store.of(self)
+        store.find(MessageChunk, MessageChunk.message == self).remove()
+        revs = [i.id for i in self.revisions]
+        store.find(
+            MessageRevisionChunk,
+            MessageRevisionChunk.message_revision_id.is_in(revs)).remove()
+        store.find(MessageRevision, MessageRevision.message == self).remove()
+        del get_property_cache(self).text_contents
+        del get_property_cache(self).chunks
+        del get_property_cache(self).revisions
+        self.date_deleted = UTC_NOW
+
 
 def get_parent_msgids(parsed_message):
     """Returns a list of message ids the mail was a reply to.
diff --git a/lib/lp/services/messages/model/messagerevision.py b/lib/lp/services/messages/model/messagerevision.py
 new file mode 100644
index 0000000..f4a1466
--- /dev/null
+++ b/lib/lp/services/messages/model/messagerevision.py
@@ -0,0 +1,92 @@
+# Copyright 2019-2021 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Message revision history."""
+
+from __future__ import absolute_import, print_function, unicode_literals
+
+__metaclass__ = type
+__all__ = [
+    'MessageRevision',
+    'MessageRevisionChunk',
+    ]
+
+import pytz
+from storm.locals import (
+    DateTime,
+    Int,
+    Reference,
+    Unicode,
+    )
+from zope.interface import implementer
+
+from lp.services.database.constants import UTC_NOW
+from lp.services.database.interfaces import IStore
+from lp.services.database.stormbase import StormBase
+from lp.services.messages.interfaces.messagerevision import (
+    IMessageRevision,
+    IMessageRevisionChunk,
+    )
+from lp.services.propertycache import (
+    cachedproperty,
+    get_property_cache,
+    )
+
+
+@implementer(IMessageRevision)
+class MessageRevision(StormBase):
+    """A historical revision of a IMessage."""
+
+    __storm_table__ = 'MessageRevision'
+
+    id = Int(primary=True)
+
+    message_id = Int(name='message', allow_none=False)
+    message = Reference(message_id, 'Message.id')
+
+    revision = Int(name='revision', allow_none=False)
+
+    date_created = DateTime(
+        name="date_created", tzinfo=pytz.UTC, allow_none=False)
+    date_deleted = DateTime(
+        name="date_deleted", tzinfo=pytz.UTC, allow_none=True)
+
+    def __init__(self, message, revision, date_created, date_deleted=None):
+        self.message = message
+        self.revision = revision
+        self.date_created = date_created
+        self.date_deleted = date_deleted
+
+    @cachedproperty
+    def chunks(self):
+        return list(IStore(self).find(
+            MessageRevisionChunk, message_revision=self))
+
+    @property
+    def content(self):
+        return '\n\n'.join(i.content for i in self.chunks)
+
+    def deleteContent(self):
+        store = IStore(self)
+        store.find(MessageRevisionChunk, message_revision=self).remove()
+        self.date_deleted = UTC_NOW
+        del get_property_cache(self).chunks
+
+
+@implementer(IMessageRevisionChunk)
+class MessageRevisionChunk(StormBase):
+    __storm_table__ = 'MessageRevisionChunk'
+
+    id = Int(primary=True)
+
+    message_revision_id = Int(name='messagerevision', allow_none=False)
+    message_revision = Reference(message_revision_id, 'MessageRevision.id')
+
+    sequence = Int(name='sequence', allow_none=False)
+
+    content = Unicode(name="content", allow_none=False)
+
+    def __init__(self, message_revision, sequence, content):
+        self.message_revision = message_revision
+        self.sequence = sequence
+        self.content = content
diff --git a/lib/lp/services/messages/tests/test_message.py b/lib/lp/services/messages/tests/test_message.py
index a9e1042..9c2bbc7 100644
--- a/lib/lp/services/messages/tests/test_message.py
+++ b/lib/lp/services/messages/tests/test_message.py
@@ -1,4 +1,4 @@
-# Copyright 2009-2018 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 __metaclass__ = type
@@ -13,15 +13,31 @@ from email.utils import (
     )
 
 import six
+from testtools.matchers import (
+    Equals,
+    Is,
+    MatchesStructure,
+    )
 import transaction
+from zope.security.interfaces import Unauthorized
+from zope.security.proxy import ProxyFactory
 
 from lp.services.compat import message_as_bytes
-from lp.services.messages.model.message import MessageSet
+from lp.services.database.interfaces import IStore
+from lp.services.database.sqlbase import get_transaction_timestamp
+from lp.services.messages.model.message import (
+    MessageChunk,
+    MessageSet,
+    )
 from lp.testing import (
     login,
+    person_logged_in,
     TestCaseWithFactory,
     )
-from lp.testing.layers import LaunchpadFunctionalLayer
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    LaunchpadFunctionalLayer,
+    )
 
 
 class TestMessageSet(TestCaseWithFactory):
@@ -169,3 +185,125 @@ class TestMessageSet(TestCaseWithFactory):
             'Treating unknown encoding "booga" as latin-1.'):
             result = MessageSet.decode(self.high_characters, 'booga')
         self.assertEqual(self.high_characters.decode('latin-1'), result)
+
+
+class TestMessageEditing(TestCaseWithFactory):
+    """Test editing scenarios for Message objects."""
+
+    layer = DatabaseFunctionalLayer
+
+    def makeMessage(self, owner=None, content=None):
+        if owner is None:
+            owner = self.factory.makePerson()
+        msg = self.factory.makeMessage(owner=owner, content=content)
+        return ProxyFactory(msg)
+
+    def test_non_owner_cannot_edit_message(self):
+        msg = self.makeMessage()
+        someone_else = self.factory.makePerson()
+        with person_logged_in(someone_else):
+            self.assertRaises(Unauthorized, getattr, msg, "editContent")
+
+    def test_msg_owner_can_edit(self):
+        owner = self.factory.makePerson()
+        msg = self.makeMessage(owner=owner, content="initial content")
+        with person_logged_in(owner):
+            msg.editContent("This is the new content")
+        self.assertEqual("This is the new content", msg.text_contents)
+        self.assertEqual(1, len(msg.revisions))
+        self.assertThat(msg.revisions[0], MatchesStructure(
+            content=Equals("initial content"),
+            revision=Equals(1),
+            message=Equals(msg),
+            date_created=Equals(msg.datecreated),
+            date_deleted=Is(None)))
+
+    def test_multiple_edits_revisions(self):
+        owner = self.factory.makePerson()
+        msg = self.makeMessage(owner=owner, content="initial content")
+        with person_logged_in(owner):
+            msg.editContent("first edit")
+            first_edit_date = msg.date_last_edited
+        self.assertEqual("first edit", msg.text_contents)
+        self.assertEqual(1, len(msg.revisions))
+        self.assertThat(msg.revisions[0], MatchesStructure(
+            content=Equals("initial content"),
+            revision=Equals(1),
+            message=Equals(msg),
+            date_created=Equals(msg.datecreated),
+            date_deleted=Is(None)))
+
+        with person_logged_in(owner):
+            msg.editContent("final form")
+        self.assertEqual("final form", msg.text_contents)
+        self.assertEqual(2, len(msg.revisions))
+        self.assertThat(msg.revisions[0], MatchesStructure(
+            content=Equals("first edit"),
+            revision=Equals(2),
+            message=Equals(msg),
+            date_created=Equals(first_edit_date),
+            date_deleted=Is(None)))
+        self.assertThat(msg.revisions[1], MatchesStructure(
+            content=Equals("initial content"),
+            revision=Equals(1),
+            message=Equals(msg),
+            date_created=Equals(msg.datecreated),
+            date_deleted=Is(None)))
+
+    def test_edit_message_with_blobs(self):
+        # Messages with blobs should keep the blobs untouched when the
+        # content is edited.
+        owner = self.factory.makePerson()
+        msg = self.makeMessage(owner=owner, content="initial content")
+        files = [self.factory.makeLibraryFileAlias(db_only=True)
+                 for _ in range(2)]
+        store = IStore(msg)
+        for seq, blob in enumerate(files):
+            store.add(MessageChunk(message=msg, sequence=seq + 2, blob=blob))
+
+        with person_logged_in(owner):
+            msg.editContent("final form")
+        self.assertThat(msg.revisions[0], MatchesStructure(
+            content=Equals("initial content"),
+            revision=Equals(1),
+            message=Equals(msg),
+            date_created=Equals(msg.datecreated),
+            date_deleted=Is(None)))
+
+        # Check that current message chunks are 3: the 2 old blobs, and the
+        # new text message.
+        self.assertEqual(3, len(msg.chunks))
+        # Make sure we avoid gaps in sequence.
+        self.assertEqual([1, 2, 3], sorted([i.sequence for i in msg.chunks]))
+        self.assertThat(msg.chunks[0], MatchesStructure(
+            content=Equals("final form"),
+            sequence=Equals(1),
+        ))
+        self.assertEqual(files, [i.blob for i in msg.chunks[1:]])
+
+        # Check revision chunks. It should be the old text message.
+        rev_chunks = msg.revisions[0].chunks
+        self.assertEqual(1, len(rev_chunks))
+        self.assertThat(rev_chunks[0], MatchesStructure(
+            sequence=Equals(1),
+            content=Equals("initial content")))
+
+    def test_non_owner_cannot_delete_message(self):
+        owner = self.factory.makePerson()
+        msg = self.makeMessage(owner=owner, content="initial content")
+        someone_else = self.factory.makePerson()
+        with person_logged_in(someone_else):
+            self.assertRaises(Unauthorized, getattr, msg, "deleteContent")
+
+    def test_delete_message(self):
+        owner = self.factory.makePerson()
+        msg = self.makeMessage(owner=owner, content="initial content")
+        with person_logged_in(owner):
+            msg.editContent("new content")
+        with person_logged_in(owner):
+            msg.deleteContent()
+        self.assertEqual('', msg.text_contents)
+        self.assertEqual(0, len(msg.chunks))
+        self.assertEqual(
+            get_transaction_timestamp(IStore(msg)), msg.date_deleted)
+        self.assertEqual(0, len(msg.revisions))
diff --git a/lib/lp/services/messages/tests/test_messagerevision.py b/lib/lp/services/messages/tests/test_messagerevision.py
 new file mode 100644
index 0000000..3b411c9
--- /dev/null
+++ b/lib/lp/services/messages/tests/test_messagerevision.py
@@ -0,0 +1,50 @@
+# Copyright 2009-2021 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+__metaclass__ = type
+
+from lp.services.database.interfaces import IStore
+from lp.services.database.sqlbase import get_transaction_timestamp
+from zope.security.interfaces import Unauthorized
+from zope.security.proxy import ProxyFactory
+
+from lp.testing import (
+    person_logged_in,
+    TestCaseWithFactory,
+    )
+from lp.testing.layers import (
+    DatabaseFunctionalLayer,
+    )
+
+
+class TestMessageRevision(TestCaseWithFactory):
+    """Test scenarios for MessageRevision objects."""
+
+    layer = DatabaseFunctionalLayer
+
+    def makeMessage(self):
+        msg = self.factory.makeMessage()
+        return ProxyFactory(msg)
+
+    def makeMessageRevision(self):
+        msg = self.makeMessage()
+        with person_logged_in(msg.owner):
+            msg.editContent('something edited #%s' % len(msg.revisions))
+        return msg.revisions[-1]
+
+    def test_non_owner_cannot_delete_message_revision_content(self):
+        rev = self.makeMessageRevision()
+        someone_else = self.factory.makePerson()
+        with person_logged_in(someone_else):
+            self.assertRaises(Unauthorized, getattr, rev, "deleteContent")
+
+    def test_msg_owner_can_delete_message_revision_content(self):
+        rev = self.makeMessageRevision()
+        msg = rev.message
+        with person_logged_in(rev.message.owner):
+            rev.deleteContent()
+        self.assertEqual(1, len(msg.revisions))
+        self.assertEqual("", rev.content)
+        self.assertEqual(0, len(rev.chunks))
+        self.assertEqual(
+            get_transaction_timestamp(IStore(rev)), rev.date_deleted)