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

Fix source_app.utils.normalize_timestamps #5724

Merged
merged 1 commit into from
Jan 20, 2021

Conversation

rmol
Copy link
Contributor

@rmol rmol commented Jan 19, 2021

Status

Ready for review

Description of Changes

Upon each new submission, source_app.utils.normalize_timestamp is called to update the timestamps of all the source's submissions. It currently uses the touch command to do this, which means if a file has gone missing, it will be recreated empty. This could mask problems that would otherwise be detected by the daily check for disconnected submissions. The function is also omitting the most recent submission from the arguments, assuming that touch is going to give the previous submissions the same timestamp as the new one, which might not be true if it's not processed instantaneously.

This changes the function to call touch with the --no-create flag and all of the source's submissions.

Testing

  • make dev
  • Log in to the source interface as a new source. Submit one message.
  • run docker exec -it securedrop-dev-0 bash
  • in the container, navigate to the source's directory under /var/lib/securedrop/store and delete the file of the message you just submitted.
  • Back in the source interface, submit another two messages, waiting a few seconds between them.
  • Back in the container shell, verify that the source's directory only contains two files (2-... and 3-...) and that their timestamps match.

Deployment

No special considerations.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make test) pass in the development container

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

Choose one of the following:

  • I have opened a PR in the docs repo for these changes, or will do so later
  • I would appreciate help with the documentation
  • These changes do not require documentation

@lgtm-com
Copy link

lgtm-com bot commented Jan 19, 2021

This pull request introduces 1 alert when merging 7add4ce into cbcc894 - view on LGTM.com

new alerts:

  • 1 for Unused import

@rmol rmol force-pushed the fix-normalize-timestamps branch from 7add4ce to 220e586 Compare January 19, 2021 23:29
@rmol rmol force-pushed the fix-normalize-timestamps branch from 220e586 to dcb7551 Compare January 20, 2021 15:57
Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Test plan passes, LGTM once CI is green

@codecov-io
Copy link

codecov-io commented Jan 20, 2021

Codecov Report

Merging #5724 (dcb7551) into develop (cbcc894) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #5724   +/-   ##
========================================
  Coverage    85.58%   85.58%           
========================================
  Files           52       52           
  Lines         3773     3773           
  Branches       471      471           
========================================
  Hits          3229     3229           
  Misses         439      439           
  Partials       105      105           
Impacted Files Coverage Δ
securedrop/source_app/utils.py 92.10% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cbcc894...dcb7551. Read the comment docs.

@zenmonkeykstop zenmonkeykstop merged commit ee61043 into develop Jan 20, 2021
@emkll emkll mentioned this pull request Jan 22, 2021
22 tasks
@kushaldas kushaldas mentioned this pull request Feb 26, 2021
27 tasks
@rmol rmol deleted the fix-normalize-timestamps branch June 23, 2021 14:09
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 this pull request may close these issues.

3 participants