From 5f7083e4361973c77ee04b96eb047bec03a8af40 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Tue, 12 Mar 2019 14:13:56 +0100 Subject: [PATCH 1/8] update db models & migrations to use sa.text(...) --- alembic/versions/2f363b3d680e_init.py | 20 ++++++++++++-------- securedrop_client/db.py | 18 +++++++++--------- 2 files changed, 21 insertions(+), 17 deletions(-) diff --git a/alembic/versions/2f363b3d680e_init.py b/alembic/versions/2f363b3d680e_init.py index 96274dfa6..0daf4ab4a 100644 --- a/alembic/versions/2f363b3d680e_init.py +++ b/alembic/versions/2f363b3d680e_init.py @@ -22,12 +22,14 @@ def upgrade(): sa.Column('id', sa.Integer(), nullable=False), sa.Column('uuid', sa.String(length=36), nullable=False), sa.Column('journalist_designation', sa.String(length=255), nullable=False), - sa.Column('document_count', sa.Integer(), server_default='0', nullable=False), - sa.Column('is_flagged', sa.Boolean(name='is_flagged'), server_default='0', nullable=True), + sa.Column('document_count', sa.Integer(), server_default=sa.text("0"), nullable=False), + sa.Column('is_flagged', sa.Boolean(name='is_flagged'), server_default=sa.text("0"), + nullable=True), sa.Column('public_key', sa.Text(), nullable=True), sa.Column('fingerprint', sa.String(length=64), nullable=True), - sa.Column('interaction_count', sa.Integer(), server_default='0', nullable=False), - sa.Column('is_starred', sa.Boolean(name='is_starred'), server_default='0', nullable=True), + sa.Column('interaction_count', sa.Integer(), server_default=sa.text("0"), nullable=False), + sa.Column('is_starred', sa.Boolean(name='is_starred'), server_default=sa.text("0"), + nullable=True), sa.Column('last_updated', sa.DateTime(), nullable=True), sa.PrimaryKeyConstraint('id', name=op.f('pk_sources')), sa.UniqueConstraint('uuid', name=op.f('uq_sources_uuid')) @@ -49,9 +51,10 @@ def upgrade(): sa.Column('filename', sa.String(length=255), nullable=False), sa.Column('size', sa.Integer(), nullable=False), sa.Column('download_url', sa.String(length=255), nullable=False), - sa.Column('is_downloaded', sa.Boolean(name='is_downloaded'), server_default='0', + sa.Column('is_downloaded', sa.Boolean(name='is_downloaded'), server_default=sa.text("0"), + nullable=False), + sa.Column('is_read', sa.Boolean(name='is_read'), server_default=sa.text("0"), nullable=False), - sa.Column('is_read', sa.Boolean(name='is_read'), server_default='0', nullable=False), sa.Column('is_decrypted', sa.Boolean(name='is_decrypted'), nullable=True), sa.Column('source_id', sa.Integer(), nullable=True), sa.ForeignKeyConstraint(['source_id'], ['sources.id'], @@ -69,9 +72,10 @@ def upgrade(): sa.Column('filename', sa.String(length=255), nullable=False), sa.Column('size', sa.Integer(), nullable=False), sa.Column('download_url', sa.String(length=255), nullable=False), - sa.Column('is_downloaded', sa.Boolean(name='is_downloaded'), server_default='0', + sa.Column('is_downloaded', sa.Boolean(name='is_downloaded'), server_default=sa.text("0"), + nullable=False), + sa.Column('is_read', sa.Boolean(name='is_read'), server_default=sa.text("0"), nullable=False), - sa.Column('is_read', sa.Boolean(name='is_read'), server_default='0', nullable=False), sa.Column('is_decrypted', sa.Boolean(name='is_decrypted'), nullable=True), sa.Column('content', sa.Text(), nullable=True), sa.Column('source_id', sa.Integer(), nullable=True), diff --git a/securedrop_client/db.py b/securedrop_client/db.py index 1f3d09939..aded2f941 100644 --- a/securedrop_client/db.py +++ b/securedrop_client/db.py @@ -1,7 +1,7 @@ import os from sqlalchemy import Boolean, Column, create_engine, DateTime, ForeignKey, Integer, String, \ - Text, MetaData, CheckConstraint + Text, MetaData, CheckConstraint, text from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import relationship, backref @@ -31,12 +31,12 @@ class Source(Base): id = Column(Integer, primary_key=True) uuid = Column(String(36), unique=True, nullable=False) journalist_designation = Column(String(255), nullable=False) - document_count = Column(Integer, server_default="0", nullable=False) - is_flagged = Column(Boolean(name='is_flagged'), server_default="0") + document_count = Column(Integer, server_default=text("0"), nullable=False) + is_flagged = Column(Boolean(name='is_flagged'), server_default=text("0")) public_key = Column(Text, nullable=True) fingerprint = Column(String(64)) - interaction_count = Column(Integer, server_default="0", nullable=False) - is_starred = Column(Boolean(name='is_starred'), server_default="0") + interaction_count = Column(Integer, server_default=text("0"), nullable=False) + is_starred = Column(Boolean(name='is_starred'), server_default=text("0")) last_updated = Column(DateTime) def __init__(self, uuid, journalist_designation, is_flagged, public_key, @@ -76,7 +76,7 @@ class Message(Base): download_url = Column(String(255), nullable=False) # This is whether the submission has been downloaded in the local database. - is_downloaded = Column(Boolean(name='is_downloaded'), nullable=False, server_default="0") + is_downloaded = Column(Boolean(name='is_downloaded'), nullable=False, server_default=text("0")) # This tracks if the file had been successfully decrypted after download. is_decrypted = Column( @@ -89,7 +89,7 @@ class Message(Base): ) # This reflects read status stored on the server. - is_read = Column(Boolean(name='is_read'), nullable=False, server_default="0") + is_read = Column(Boolean(name='is_read'), nullable=False, server_default=text("0")) content = Column( Text, @@ -118,7 +118,7 @@ class File(Base): download_url = Column(String(255), nullable=False) # This is whether the submission has been downloaded in the local database. - is_downloaded = Column(Boolean(name='is_downloaded'), nullable=False, server_default="0") + is_downloaded = Column(Boolean(name='is_downloaded'), nullable=False, server_default=text("0")) # This tracks if the file had been successfully decrypted after download. is_decrypted = Column( @@ -129,7 +129,7 @@ class File(Base): ) # This reflects read status stored on the server. - is_read = Column(Boolean(name='is_read'), nullable=False, server_default="0") + is_read = Column(Boolean(name='is_read'), nullable=False, server_default=text("0")) source_id = Column(Integer, ForeignKey('sources.id')) source = relationship("Source", From fce3808e40f62b3ddc3a1966814a3b73c9ee71cd Mon Sep 17 00:00:00 2001 From: heartsucker Date: Tue, 12 Mar 2019 14:20:47 +0100 Subject: [PATCH 2/8] ensure constraint name matches convention --- alembic/versions/2f363b3d680e_init.py | 2 +- securedrop_client/db.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/alembic/versions/2f363b3d680e_init.py b/alembic/versions/2f363b3d680e_init.py index 0daf4ab4a..a28a5f571 100644 --- a/alembic/versions/2f363b3d680e_init.py +++ b/alembic/versions/2f363b3d680e_init.py @@ -84,7 +84,7 @@ def upgrade(): sa.PrimaryKeyConstraint('id', name=op.f('pk_messages')), sa.UniqueConstraint('uuid', name=op.f('uq_messages_uuid')), sa.CheckConstraint('CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END', - name='messages_compare_download_vs_content'), + name=op.f('ck_message_compare_download_vs_content')), sa.CheckConstraint('CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END', name='messages_compare_is_downloaded_vs_is_decrypted'), sa.CheckConstraint('CASE WHEN is_decrypted = 0 THEN content IS NULL ELSE 1 END', diff --git a/securedrop_client/db.py b/securedrop_client/db.py index aded2f941..f8f157599 100644 --- a/securedrop_client/db.py +++ b/securedrop_client/db.py @@ -95,7 +95,7 @@ class Message(Base): Text, # this check contraint ensures the state of the DB is what one would expect CheckConstraint('CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END', - name='messages_compare_download_vs_content') + name='ck_message_compare_download_vs_content') ) source_id = Column(Integer, ForeignKey('sources.id')) From 0274df8c284eae918b506f33ffa5a3194e8c4702 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Tue, 12 Mar 2019 14:22:09 +0100 Subject: [PATCH 3/8] mark source_id foreign key columns as not null --- alembic/versions/2f363b3d680e_init.py | 6 +++--- securedrop_client/db.py | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/alembic/versions/2f363b3d680e_init.py b/alembic/versions/2f363b3d680e_init.py index a28a5f571..b1fe94c67 100644 --- a/alembic/versions/2f363b3d680e_init.py +++ b/alembic/versions/2f363b3d680e_init.py @@ -56,7 +56,7 @@ def upgrade(): sa.Column('is_read', sa.Boolean(name='is_read'), server_default=sa.text("0"), nullable=False), sa.Column('is_decrypted', sa.Boolean(name='is_decrypted'), nullable=True), - sa.Column('source_id', sa.Integer(), nullable=True), + sa.Column('source_id', sa.Integer(), nullable=False), sa.ForeignKeyConstraint(['source_id'], ['sources.id'], name=op.f('fk_files_source_id_sources')), sa.PrimaryKeyConstraint('id', name=op.f('pk_files')), @@ -78,7 +78,7 @@ def upgrade(): nullable=False), sa.Column('is_decrypted', sa.Boolean(name='is_decrypted'), nullable=True), sa.Column('content', sa.Text(), nullable=True), - sa.Column('source_id', sa.Integer(), nullable=True), + sa.Column('source_id', sa.Integer(), nullable=False), sa.ForeignKeyConstraint(['source_id'], ['sources.id'], name=op.f('fk_messages_source_id_sources')), sa.PrimaryKeyConstraint('id', name=op.f('pk_messages')), @@ -95,7 +95,7 @@ def upgrade(): 'replies', sa.Column('id', sa.Integer(), nullable=False), sa.Column('uuid', sa.String(length=36), nullable=False), - sa.Column('source_id', sa.Integer(), nullable=True), + sa.Column('source_id', sa.Integer(), nullable=False), sa.Column('journalist_id', sa.Integer(), nullable=True), sa.Column('filename', sa.String(length=255), nullable=False), sa.Column('size', sa.Integer(), nullable=True), diff --git a/securedrop_client/db.py b/securedrop_client/db.py index f8f157599..4cde754cb 100644 --- a/securedrop_client/db.py +++ b/securedrop_client/db.py @@ -98,7 +98,7 @@ class Message(Base): name='ck_message_compare_download_vs_content') ) - source_id = Column(Integer, ForeignKey('sources.id')) + source_id = Column(Integer, ForeignKey('sources.id'), nullable=False) source = relationship("Source", backref=backref("messages", order_by=id, cascade="delete")) @@ -131,7 +131,7 @@ class File(Base): # This reflects read status stored on the server. is_read = Column(Boolean(name='is_read'), nullable=False, server_default=text("0")) - source_id = Column(Integer, ForeignKey('sources.id')) + source_id = Column(Integer, ForeignKey('sources.id'), nullable=False) source = relationship("Source", backref=backref("files", order_by=id, cascade="delete")) @@ -146,7 +146,7 @@ class Reply(Base): id = Column(Integer, primary_key=True) uuid = Column(String(36), unique=True, nullable=False) - source_id = Column(Integer, ForeignKey('sources.id')) + source_id = Column(Integer, ForeignKey('sources.id'), nullable=False) source = relationship("Source", backref=backref("replies", order_by=id, cascade="delete")) From cd09ee84a054c5e373a8406389dce4661266f521 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Tue, 12 Mar 2019 14:37:58 +0100 Subject: [PATCH 4/8] add file_counter columns, update init function --- securedrop_client/db.py | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/securedrop_client/db.py b/securedrop_client/db.py index 4cde754cb..92e29ed87 100644 --- a/securedrop_client/db.py +++ b/securedrop_client/db.py @@ -1,7 +1,7 @@ import os from sqlalchemy import Boolean, Column, create_engine, DateTime, ForeignKey, Integer, String, \ - Text, MetaData, CheckConstraint, text + Text, MetaData, CheckConstraint, text, UniqueConstraint from sqlalchemy.ext.declarative import declarative_base from sqlalchemy.orm import relationship, backref @@ -68,10 +68,14 @@ def collection(self): class Message(Base): __tablename__ = 'messages' + __table_args__ = ( + UniqueConstraint('source_id', 'file_counter', name='uq_messages_source_id_file_counter'), + ) id = Column(Integer, primary_key=True) uuid = Column(String(36), unique=True, nullable=False) filename = Column(String(255), nullable=False) + file_counter = Column(Integer, nullable=False) size = Column(Integer, nullable=False) download_url = Column(String(255), nullable=False) @@ -103,6 +107,13 @@ class Message(Base): backref=backref("messages", order_by=id, cascade="delete")) + def __init__(self, **kwargs) -> None: + if 'file_counter' in kwargs: + raise TypeError('Cannot manually set file_counter') + filename = kwargs['filename'] + kwargs['file_counter'] = int(filename.split('-')[0]) + super().__init__(**kwargs) + def __repr__(self): return ''.format(self.filename) @@ -110,10 +121,14 @@ def __repr__(self): class File(Base): __tablename__ = 'files' + __table_args__ = ( + UniqueConstraint('source_id', 'file_counter', name='uq_messages_source_id_file_counter'), + ) id = Column(Integer, primary_key=True) uuid = Column(String(36), unique=True, nullable=False) filename = Column(String(255), nullable=False) + file_counter = Column(Integer, nullable=False) size = Column(Integer, nullable=False) download_url = Column(String(255), nullable=False) @@ -136,6 +151,13 @@ class File(Base): backref=backref("files", order_by=id, cascade="delete")) + def __init__(self, **kwargs) -> None: + if 'file_counter' in kwargs: + raise TypeError('Cannot manually set file_counter') + filename = kwargs['filename'] + kwargs['file_counter'] = int(filename.split('-')[0]) + super().__init__(**kwargs) + def __repr__(self): return ''.format(self.filename) @@ -143,6 +165,9 @@ def __repr__(self): class Reply(Base): __tablename__ = 'replies' + __table_args__ = ( + UniqueConstraint('source_id', 'file_counter', name='uq_messages_source_id_file_counter'), + ) id = Column(Integer, primary_key=True) uuid = Column(String(36), unique=True, nullable=False) @@ -156,6 +181,7 @@ class Reply(Base): "User", backref=backref('replies', order_by=id)) filename = Column(String(255), nullable=False) + file_counter = Column(Integer, nullable=False) size = Column(Integer) # This is whether the reply has been downloaded in the local database. @@ -178,6 +204,13 @@ class Reply(Base): nullable=True, ) + def __init__(self, **kwargs) -> None: + if 'file_counter' in kwargs: + raise TypeError('Cannot manually set file_counter') + filename = kwargs['filename'] + kwargs['file_counter'] = int(filename.split('-')[0]) + super().__init__(**kwargs) + def __repr__(self): return ''.format(self.filename) From 5b698c3370c9c0f7cea02f7d40401df5b725e81d Mon Sep 17 00:00:00 2001 From: heartsucker Date: Tue, 12 Mar 2019 14:38:17 +0100 Subject: [PATCH 5/8] update migration to include file_counter columns --- alembic/versions/2f363b3d680e_init.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/alembic/versions/2f363b3d680e_init.py b/alembic/versions/2f363b3d680e_init.py index b1fe94c67..6d1ac7fec 100644 --- a/alembic/versions/2f363b3d680e_init.py +++ b/alembic/versions/2f363b3d680e_init.py @@ -32,7 +32,7 @@ def upgrade(): nullable=True), sa.Column('last_updated', sa.DateTime(), nullable=True), sa.PrimaryKeyConstraint('id', name=op.f('pk_sources')), - sa.UniqueConstraint('uuid', name=op.f('uq_sources_uuid')) + sa.UniqueConstraint('uuid', name=op.f('uq_sources_uuid')), ) op.create_table( @@ -41,7 +41,7 @@ def upgrade(): sa.Column('uuid', sa.String(length=36), nullable=False), sa.Column('username', sa.String(length=255), nullable=False), sa.PrimaryKeyConstraint('id', name=op.f('pk_users')), - sa.UniqueConstraint('uuid', name=op.f('uq_users_uuid')) + sa.UniqueConstraint('uuid', name=op.f('uq_users_uuid')), ) op.create_table( @@ -49,6 +49,7 @@ def upgrade(): sa.Column('id', sa.Integer(), nullable=False), sa.Column('uuid', sa.String(length=36), nullable=False), sa.Column('filename', sa.String(length=255), nullable=False), + sa.Column('file_counter', sa.Integer(), nullable=False), sa.Column('size', sa.Integer(), nullable=False), sa.Column('download_url', sa.String(length=255), nullable=False), sa.Column('is_downloaded', sa.Boolean(name='is_downloaded'), server_default=sa.text("0"), @@ -60,6 +61,7 @@ def upgrade(): sa.ForeignKeyConstraint(['source_id'], ['sources.id'], name=op.f('fk_files_source_id_sources')), sa.PrimaryKeyConstraint('id', name=op.f('pk_files')), + sa.UniqueConstraint('source_id', 'file_counter', name='uq_messages_source_id_file_counter'), sa.UniqueConstraint('uuid', name=op.f('uq_files_uuid')), sa.CheckConstraint('CASE WHEN is_downloaded = 0 THEN is_decrypted IS NULL ELSE 1 END', name='files_compare_is_downloaded_vs_is_decrypted'), @@ -70,6 +72,7 @@ def upgrade(): sa.Column('id', sa.Integer(), nullable=False), sa.Column('uuid', sa.String(length=36), nullable=False), sa.Column('filename', sa.String(length=255), nullable=False), + sa.Column('file_counter', sa.Integer(), nullable=False), sa.Column('size', sa.Integer(), nullable=False), sa.Column('download_url', sa.String(length=255), nullable=False), sa.Column('is_downloaded', sa.Boolean(name='is_downloaded'), server_default=sa.text("0"), @@ -82,6 +85,7 @@ def upgrade(): sa.ForeignKeyConstraint(['source_id'], ['sources.id'], name=op.f('fk_messages_source_id_sources')), sa.PrimaryKeyConstraint('id', name=op.f('pk_messages')), + sa.UniqueConstraint('source_id', 'file_counter', name='uq_messages_source_id_file_counter'), sa.UniqueConstraint('uuid', name=op.f('uq_messages_uuid')), sa.CheckConstraint('CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END', name=op.f('ck_message_compare_download_vs_content')), @@ -98,6 +102,7 @@ def upgrade(): sa.Column('source_id', sa.Integer(), nullable=False), sa.Column('journalist_id', sa.Integer(), nullable=True), sa.Column('filename', sa.String(length=255), nullable=False), + sa.Column('file_counter', sa.Integer(), nullable=False), sa.Column('size', sa.Integer(), nullable=True), sa.Column('content', sa.Text(), nullable=True), sa.Column('is_downloaded', sa.Boolean(name='is_downloaded'), nullable=True), @@ -107,6 +112,7 @@ def upgrade(): sa.ForeignKeyConstraint(['source_id'], ['sources.id'], name=op.f('fk_replies_source_id_sources')), sa.PrimaryKeyConstraint('id', name=op.f('pk_replies')), + sa.UniqueConstraint('source_id', 'file_counter', name='uq_messages_source_id_file_counter'), sa.UniqueConstraint('uuid', name=op.f('uq_replies_uuid')), sa.CheckConstraint('CASE WHEN is_downloaded = 0 THEN content IS NULL ELSE 1 END', name='replies_compare_download_vs_content'), From f67187f0f8e4e1a8929c10e62f2226badc604f46 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Tue, 12 Mar 2019 14:49:53 +0100 Subject: [PATCH 6/8] updated tests to meet new filename constraint --- tests/gui/test_main.py | 2 +- tests/gui/test_widgets.py | 14 +++++++------- tests/test_logic.py | 16 ++++++++-------- tests/test_message_sync.py | 10 +++++----- tests/test_models.py | 6 +++--- tests/test_storage.py | 8 ++++---- 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/tests/gui/test_main.py b/tests/gui/test_main.py index c414afbfb..48198a67d 100644 --- a/tests/gui/test_main.py +++ b/tests/gui/test_main.py @@ -249,7 +249,7 @@ def test_conversation_pending_message(mocker): mock_source.journalistic_designation = 'Testy McTestface' msg_uuid = str(uuid4()) - message = Message(source=mock_source, uuid=msg_uuid, size=123, filename="test.msg.gpg", + message = Message(source=mock_source, uuid=msg_uuid, size=123, filename="1-test.msg.gpg", download_url='http://test/test', is_downloaded=False) mock_source.collection = [message] diff --git a/tests/gui/test_widgets.py b/tests/gui/test_widgets.py index 87ffa1b34..69a492684 100644 --- a/tests/gui/test_widgets.py +++ b/tests/gui/test_widgets.py @@ -339,9 +339,9 @@ def test_SourceWidget_delete_source_when_user_chooses_cancel(mocker): mock_source.submissions = [] submission_files = ( - "submission_1-msg.gpg", - "submission_2-msg.gpg", - "submission_3-doc.gpg", + "1-submission-msg.gpg", + "2-submission-msg.gpg", + "3-submission-doc.gpg", ) for filename in submission_files: submission = mocker.MagicMock() @@ -682,7 +682,7 @@ def test_FileWidget_init_left(mocker): """ mock_controller = mocker.MagicMock() source = factory.Source() - message = db.Message(source=source, uuid='uuid', size=123, filename='mah-reply.gpg', + message = db.Message(source=source, uuid='uuid', size=123, filename='1-mah-reply.gpg', download_url='http://mah-server/mah-reply-url', is_downloaded=True) fw = FileWidget(source, message, mock_controller, align='left') @@ -700,7 +700,7 @@ def test_FileWidget_init_right(mocker): """ mock_controller = mocker.MagicMock() source = factory.Source() - message = db.Message(source=source, uuid='uuid', size=123, filename='mah-reply.gpg', + message = db.Message(source=source, uuid='uuid', size=123, filename='1-mah-reply.gpg', download_url='http://mah-server/mah-reply-url', is_downloaded=True) fw = FileWidget(source, message, mock_controller, align='right') @@ -717,7 +717,7 @@ def test_FileWidget_mousePressEvent_download(mocker): """ mock_controller = mocker.MagicMock() source = factory.Source() - file_ = db.File(source=source, uuid='uuid', size=123, filename='mah-reply.gpg', + file_ = db.File(source=source, uuid='uuid', size=123, filename='1-mah-reply.gpg', download_url='http://mah-server/mah-reply-url', is_downloaded=False) fw = FileWidget(source, file_, mock_controller) @@ -731,7 +731,7 @@ def test_FileWidget_mousePressEvent_open(mocker): """ mock_controller = mocker.MagicMock() source = factory.Source() - file_ = db.File(source=source, uuid='uuid', size=123, filename='mah-reply.gpg', + file_ = db.File(source=source, uuid='uuid', size=123, filename='1-mah-reply.gpg', download_url='http://mah-server/mah-reply-url', is_downloaded=True) fw = FileWidget(source, file_, mock_controller) diff --git a/tests/test_logic.py b/tests/test_logic.py index 7f81af59d..549e4765d 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -951,7 +951,7 @@ def test_Client_on_file_download_Submission(homedir, config, mocker): mock_session = mocker.MagicMock() cl = Client('http://localhost', mock_gui, mock_session, homedir) source = factory.Source() - file_ = db.File(source=source, uuid='uuid', size=1234, filename='myfile.doc.gpg', + file_ = db.File(source=source, uuid='uuid', size=1234, filename='1-myfile.doc.gpg', download_url='http://myserver/myfile', is_downloaded=False) cl.call_api = mocker.MagicMock() cl.api = mocker.MagicMock() @@ -974,7 +974,7 @@ def test_Client_on_file_downloaded_success(homedir, config, mocker): cl = Client('http://localhost', mock_gui, mock_session, homedir) cl.update_sources = mocker.MagicMock() cl.api_runner = mocker.MagicMock() - test_filename = "my-file-location-msg.gpg" + test_filename = "1-my-file-location-msg.gpg" test_object_uuid = 'uuid-of-downloaded-object' cl.call_reset = mocker.MagicMock() result_data = ('this-is-a-sha256-sum', test_filename) @@ -1001,7 +1001,7 @@ def test_Client_on_file_downloaded_api_failure(homedir, config, mocker): cl = Client('http://localhost', mock_gui, mock_session, homedir) cl.update_sources = mocker.MagicMock() cl.api_runner = mocker.MagicMock() - test_filename = "my-file-location-msg.gpg" + test_filename = "1-my-file-location-msg.gpg" cl.api_runner.result = ("", test_filename) cl.call_reset = mocker.MagicMock() cl.set_status = mocker.MagicMock() @@ -1023,7 +1023,7 @@ def test_Client_on_file_downloaded_decrypt_failure(homedir, config, mocker): cl = Client('http://localhost', mock_gui, mock_session, homedir) cl.update_sources = mocker.MagicMock() cl.api_runner = mocker.MagicMock() - test_filename = "my-file-location-msg.gpg" + test_filename = "1-my-file-location-msg.gpg" cl.api_runner.result = ("", test_filename) cl.set_status = mocker.MagicMock() result_data = ('this-is-a-sha256-sum', test_filename) @@ -1053,7 +1053,7 @@ def test_Client_on_file_download_user_not_signed_in(homedir, config, mocker): mock_session = mocker.MagicMock() cl = Client('http://localhost', mock_gui, mock_session, homedir) source = factory.Source() - file_ = db.File(source=source, uuid='uuid', size=1234, filename='myfile.doc.gpg', + file_ = db.File(source=source, uuid='uuid', size=1234, filename='1-myfile.doc.gpg', download_url='http://myserver/myfile', is_downloaded=False) cl.on_action_requiring_login = mocker.MagicMock() cl.api = None @@ -1074,7 +1074,7 @@ def test_Client_on_download_timeout(homedir, config, mocker): cl.update_sources = mocker.MagicMock() cl.api_runner = mocker.MagicMock() current_object = mocker.MagicMock() - test_filename = "my-file-location-msg.gpg" + test_filename = "1-my-file-location-msg.gpg" cl.api_runner.result = ("", test_filename) cl.call_reset = mocker.MagicMock() cl.set_status = mocker.MagicMock() @@ -1097,7 +1097,7 @@ def test_Client_on_file_download_Reply(homedir, config, mocker): reply = db.Reply(uuid='reply-uuid', journalist=journalist, source=source, - filename='my-reply.gpg', + filename='1-my-reply.gpg', size=123) # Not a sdclientapi.Submission cl.call_api = mocker.MagicMock() cl.api = mocker.MagicMock() @@ -1123,7 +1123,7 @@ def test_Client_on_file_open(homedir, config, mocker): cl = Client('http://localhost', mock_gui, mock_session, homedir) cl.proxy = True mock_submission = mocker.MagicMock() - mock_submission.filename = 'test.pdf' + mock_submission.filename = '1-test.pdf' mock_subprocess = mocker.MagicMock() mock_process = mocker.MagicMock(return_value=mock_subprocess) mocker.patch('securedrop_client.logic.QProcess', mock_process) diff --git a/tests/test_message_sync.py b/tests/test_message_sync.py index c6473adcd..75048bdc7 100644 --- a/tests/test_message_sync.py +++ b/tests/test_message_sync.py @@ -32,7 +32,7 @@ def test_MessageSync_init(mocker): def test_MessageSync_run_success(mocker, source): """Test when a message successfully downloads and decrypts.""" - submission = Message(source=source['source'], uuid='uuid2', filename='filename', + submission = Message(source=source['source'], uuid='uuid2', filename='1-filename', download_url='http://test.net') # mock the fetching of submissions @@ -70,7 +70,7 @@ def test_MessageSync_run_success(mocker, source): def test_MessageSync_run_decryption_error(mocker, source): """Test when a message successfully downloads, but does not successfully decrypt.""" - submission = File(source=source['source'], uuid='uuid1', filename='filename', + submission = File(source=source['source'], uuid='uuid1', filename='1-filename', download_url='http://test.net') # mock the fetching of submissions @@ -136,7 +136,7 @@ def test_MessageSync_exception(homedir, config, mocker): def test_MessageSync_run_failure(mocker): submission = mocker.MagicMock() submission.download_url = "http://foo" - submission.filename = "foo.gpg" + submission.filename = "1-foo.gpg" # mock the fetching of submissions mocker.patch('securedrop_client.storage.find_new_messages', return_value=[submission]) @@ -162,7 +162,7 @@ def test_ReplySync_run_success(mocker): reply = mocker.MagicMock() reply.uuid = 'mock id' reply.download_url = "http://foo" - reply.filename = "foo.gpg" + reply.filename = "1-foo.gpg" api = mocker.MagicMock() home = "/home/user/.sd" @@ -219,7 +219,7 @@ def test_ReplySync_exception(mocker): def test_ReplySync_run_failure(mocker): reply = mocker.MagicMock() reply.download_url = "http://foo" - reply.filename = "foo.gpg" + reply.filename = "1-foo.gpg" # mock finding new replies mocker.patch('securedrop_client.storage.find_new_replies', return_value=[reply]) diff --git a/tests/test_models.py b/tests/test_models.py index 1be8a4d33..1c9585388 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -14,14 +14,14 @@ def test_string_representation_of_source(): def test_string_representation_of_message(): source = factory.Source() - msg = Message(source=source, uuid="test", size=123, filename="test.docx", + msg = Message(source=source, uuid="test", size=123, filename="1-test.docx", download_url='http://test/test') msg.__repr__() def test_string_representation_of_file(): source = factory.Source() - file_ = File(source=source, uuid="test", size=123, filename="test.docx", + file_ = File(source=source, uuid="test", size=123, filename="1-test.docx", download_url='http://test/test') file_.__repr__() @@ -29,7 +29,7 @@ def test_string_representation_of_file(): def test_string_representation_of_reply(): user = User('hehe') source = factory.Source() - reply = Reply(source=source, journalist=user, filename="reply.gpg", + reply = Reply(source=source, journalist=user, filename="1-reply.gpg", size=1234, uuid='test') reply.__repr__() diff --git a/tests/test_storage.py b/tests/test_storage.py index c90f8455b..8accc9edb 100644 --- a/tests/test_storage.py +++ b/tests/test_storage.py @@ -39,7 +39,7 @@ def make_remote_submission(source_uuid): generate a valid URL. """ source_url = '/api/v1/sources/{}'.format(source_uuid) - return Submission(download_url='test', filename='submission.filename', + return Submission(download_url='test', filename='1-submission.filename', is_read=False, size=123, source_url=source_url, submission_url='test', uuid=str(uuid.uuid4())) @@ -51,7 +51,7 @@ def make_remote_reply(source_uuid, journalist_uuid='testymctestface'): generate a valid URL. """ source_url = '/api/v1/sources/{}'.format(source_uuid) - return Reply(filename='reply.filename', journalist_uuid=journalist_uuid, + return Reply(filename='1-reply.filename', journalist_uuid=journalist_uuid, journalist_username='test', is_deleted_by_source=False, reply_url='test', size=1234, source_url=source_url, uuid=str(uuid.uuid4())) @@ -130,8 +130,8 @@ def test_update_local_storage(homedir, mocker): the state of the local database are called with the necessary data. """ remote_source = make_remote_source() - remote_message = mocker.Mock(filename='foo.msg.gpg') - remote_file = mocker.Mock(filename='foo.gpg') + remote_message = mocker.Mock(filename='1-foo.msg.gpg') + remote_file = mocker.Mock(filename='2-foo.gpg') remote_submissions = [remote_message, remote_file] remote_reply = mocker.MagicMock() # Some local source, submission and reply objects from the local database. From 37fe0aab4eaf51956775720c0cf50adc9e1b9e9f Mon Sep 17 00:00:00 2001 From: heartsucker Date: Tue, 12 Mar 2019 14:51:30 +0100 Subject: [PATCH 7/8] test for file/message/reply init --- tests/test_models.py | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/tests/test_models.py b/tests/test_models.py index 1c9585388..388f64081 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -1,3 +1,5 @@ +import pytest + from tests import factory from securedrop_client.db import Reply, File, Message, User @@ -52,3 +54,42 @@ def test_source_collection(): assert source.collection[0] == reply assert source.collection[1] == file_ assert source.collection[2] == message + + +def test_file_init(): + ''' + Check that: + - We can't pass the file_counter attribute + - The file_counter attribute is see correctly based off the filename + ''' + with pytest.raises(TypeError): + File(file_counter=1) + + f = File(filename="1-foo") + assert f.file_counter == 1 + + +def test_message_init(): + ''' + Check that: + - We can't pass the file_counter attribute + - The file_counter attribute is see correctly based off the filename + ''' + with pytest.raises(TypeError): + Message(file_counter=1) + + m = Message(filename="1-foo") + assert m.file_counter == 1 + + +def test_reply_init(): + ''' + Check that: + - We can't pass the file_counter attribute + - The file_counter attribute is see correctly based off the filename + ''' + with pytest.raises(TypeError): + Reply(file_counter=1) + + r = Reply(filename="1-foo") + assert r.file_counter == 1 From da8a2c75a74c198824ef8dc24aa8d437658e3c15 Mon Sep 17 00:00:00 2001 From: heartsucker Date: Tue, 12 Mar 2019 15:05:36 +0100 Subject: [PATCH 8/8] sort by file_counter in collections --- securedrop_client/db.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/securedrop_client/db.py b/securedrop_client/db.py index 92e29ed87..93f6aed31 100644 --- a/securedrop_client/db.py +++ b/securedrop_client/db.py @@ -61,7 +61,7 @@ def collection(self): collection.extend(self.messages) collection.extend(self.files) collection.extend(self.replies) - collection.sort(key=lambda x: int(x.filename.split('-')[0])) + collection.sort(key=lambda x: x.file_counter) return collection