uisautomation / media-webapp

Web application for the University Media Platform
https://alpha.media.cam.ac.uk
MIT License
4 stars 7 forks source link

ensure media upload endpoints are one-off #392

Open rjw57 opened 5 years ago

rjw57 commented 5 years ago

JWP media upload endpoints are one-use-only URLs. Unfortunately we allowed the same URL to be retrieved multiple times from the upload endpoint which meant that a client may try to upload a media item multiple times to the same URL.

Fix this by making the /media/{id}/upload endpoint a PUT-only endpoint. If the upload endpoint URL exists in the database to begin with, we return it and delete it from the database. If the endpoint does not exist, we create one first.

This involves a bit of an unnecessary write and then delete from the database in the case where the endpoint does not exist but for the common case of "create media item" then "get upload endpoint", it Does The Right Thing (TM).

Closes #367

codecov[bot] commented 5 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@76a8891). Click here to learn what that means. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #392   +/-   ##
=========================================
  Coverage          ?   91.32%           
=========================================
  Files             ?       43           
  Lines             ?     1949           
  Branches          ?        0           
=========================================
  Hits              ?     1780           
  Misses            ?      169           
  Partials          ?        0
Impacted Files Coverage Δ
api/serializers.py 97.87% <100%> (ø)
api/views.py 95.18% <100%> (ø)

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 76a8891...3802a13. Read the comment docs.

rjw57 commented 5 years ago

If the PUT succeeds after JWP has finished processing, I'd be inclined to call that good enough to fix the issue for the moment. We're likely to migrate away from JWP so re-woking all the API plumbing to support it is probably left for another issue.