Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Store messages/replies in the database in new content column #217

Closed
redshiftzero opened this issue Dec 20, 2018 · 3 comments · Fixed by #277
Closed

Store messages/replies in the database in new content column #217

redshiftzero opened this issue Dec 20, 2018 · 3 comments · Fixed by #277
Assignees

Comments

@redshiftzero
Copy link
Contributor

redshiftzero commented Dec 20, 2018

Part of #231.

Right now we store the text of the messages/files as files on disk. The proposal here is to store message and reply content in the local sqlite database instead of a file on disk.

Advantages

The main advantage is cleaner separation of data handling and GUI logic: this change enables us to move more data-layer functionality into methods on the database models (out of data.py and out of storage.py into one place: the database models). The issue with not doing this is that we then either have to use a clever solution like what is described in #197 or we have to pass around the data_dir attribute into parts of the gui code (#212) so that we can then pass the data_dir to another function (in storage.py used for fetching the content in a file).

This means that functionality like last_activity_summary_text gets implemented as a method on the Source class. We get the content via item.content elsewhere in the code (clean/simple). The gui parts of the code don't need to know what data_dir is.

(This also is a step in the right direction if we ever want to enable people to search through text in the client application)

Disadvantages

Larger database, but at the scale of SecureDrop this should not be a problem (i.e. with ~100 sources the database should be only a few MB or tens of MB).

@heartsucker
Copy link
Contributor

One clarifying question: When you say "files" above, do you also mean non-message submissions such as PDFs or whatnot? I assume not because that would probably break sqlite 😅

I do agree that this is probably a better way to do this. This would likely need a boolean column to know whether the reply is fully decrypted.

@redshiftzero
Copy link
Contributor Author

One clarifying question: When you say "files" above, do you also mean non-message submissions such as PDFs or whatnot? I assume not because that would probably break sqlite 😅

Yep, only for messages and replies

@heartsucker
Copy link
Contributor

Recommend merging #240 before this is implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants