Fixups for admin SponsorLogo upload. (pypi#9794)

* ensure prefix is applied for GCSSponsorLogoStorage

* lint

* sponsor logos: deduplicate upload code, fingerprint uploads

* my first walrus

* fix coverage

tests/unit/admin/test_services.py

@@ -90,6 +90,25 @@ def test_stores_file(self, tmpdir):
         assert blob.make_public.calls == [pretend.call()]
         assert blob.upload_from_filename.calls == [pretend.call(filename)]
 
+    def test_stores_file_with_prefix(self, tmpdir):
+        filename = str(tmpdir.join("testfile.txt"))
+        with open(filename, "wb") as fp:
+            fp.write(b"Test File!")
+
+        blob = pretend.stub(
+            upload_from_filename=pretend.call_recorder(lambda file_path: None),
+            make_public=pretend.call_recorder(lambda: None),
+            public_url="http://files/sponsorlogos/thelogo.png",
+        )
+        bucket = pretend.stub(blob=pretend.call_recorder(lambda path: blob))
+        storage = GCSSponsorLogoStorage(bucket, prefix="sponsorlogos")
+        result = storage.store("foo/bar.txt", filename)
+
+        assert result == "http://files/sponsorlogos/thelogo.png"
+        assert bucket.blob.calls == [pretend.call("sponsorlogos/foo/bar.txt")]
+        assert blob.make_public.calls == [pretend.call()]
+        assert blob.upload_from_filename.calls == [pretend.call(filename)]
+
     def test_stores_metadata(self, tmpdir):
         filename = str(tmpdir.join("testfile.txt"))
         with open(filename, "wb") as fp:

tests/unit/admin/views/test_sponsors.py

@@ -10,6 +10,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 import io
+import secrets
 import uuid
 
 from cgi import FieldStorage
@@ -129,12 +130,13 @@ def test_404_if_sponsor_does_not_exist(self, db_request):
         with pytest.raises(HTTPNotFound):
             views.edit_sponsor(db_request)
 
-    def test_update_sponsor(self, db_request):
+    def test_update_sponsor(self, monkeypatch, db_request):
         sponsor = SponsorFactory.create()
         form = views.SponsorForm(MultiDict({}), sponsor)
         data = form.data.copy()
         data["name"] = "New Name"
         data["white_logo"] = WHITE_LOGO_FILE
+        data["color_logo"] = COLOR_LOGO_FILE
         db_request.matchdict["sponsor_id"] = sponsor.id
         db_request.method = "POST"
         db_request.POST = MultiDict(data)
@@ -155,15 +157,22 @@ def test_update_sponsor(self, db_request):
             }.get(svc)
         )
 
+        monkeypatch.setattr(secrets, "token_urlsafe", lambda x: "deadbeef")
+
         resp = views.edit_sponsor(db_request)
+
         db_sponsor = db_request.db.query(Sponsor).filter(Sponsor.id == sponsor.id).one()
 
         assert resp.status_code == 303
         assert resp.location == f"/admin/sponsors/{sponsor.id}/"
         assert db_sponsor.name == "New Name"
         assert (
             db_sponsor.white_logo_url
-            == "http://files/sponsorlogos/new-name-white-logo.png"
+            == "http://files/sponsorlogos/new-name-white-logo-deadbeef.png"
+        )
+        assert (
+            db_sponsor.color_logo_url
+            == "http://files/sponsorlogos/new-name-color-logo-deadbeef.png"
         )
         assert db_request.session.flash.calls == [
             pretend.call("Sponsor updated", queue="success")

warehouse/admin/services.py

@@ -81,6 +81,8 @@ def create_service(cls, context, request):
         )
     )
     def store(self, path, file_path, *, meta=None):
+        if self.prefix is not None:
+            path = os.path.join(self.prefix, path)
         blob = self.bucket.blob(path)
         if meta is not None:
             blob.metadata = meta

warehouse/admin/views/sponsors.py

@@ -11,6 +11,7 @@
 # limitations under the License.
 
 import os
+import secrets
 import tempfile
 
 import wtforms
@@ -102,22 +103,17 @@ def sponsor_list(request):
     return {"sponsors": sponsors}
 
 
-def _handle_images(request, form):
+def _upload_image(image_name, request, form):
     sponsor_name = slugify(form.name.data)
-    if request.POST.get("white_logo") not in [None, b""]:
+    if request.POST.get(image_name) not in [None, b""]:
         with tempfile.NamedTemporaryFile() as fp:
-            fp.write(request.POST["white_logo"].file.read())
+            fp.write(request.POST[image_name].file.read())
             storage = request.find_service(ISponsorLogoStorage)
-            extension = os.path.splitext(request.POST["white_logo"].filename)[-1]
-            filename = f"{sponsor_name}-white-logo{extension}"
-            form.white_logo_url.data = storage.store(filename, fp.name)
-    if request.POST.get("color_logo") not in [None, b""]:
-        with tempfile.NamedTemporaryFile() as fp:
-            storage = request.find_service(ISponsorLogoStorage)
-            fp.write(request.POST["color_logo"].file.read())
-            extension = os.path.splitext(request.POST["color_logo"].filename)[-1]
-            filename = f"{sponsor_name}-color-logo{extension}"
-            form.color_logo_url.data = storage.store(filename, fp.name)
+            extension = os.path.splitext(request.POST[image_name].filename)[-1]
+            fingerprint = secrets.token_urlsafe(6)
+            filename = f"{sponsor_name}-{slugify(image_name)}-{fingerprint}{extension}"
+            return storage.store(filename, fp.name)
+    return ""
 
 
 @view_config(
@@ -148,7 +144,10 @@ def edit_sponsor(request):
     form = SponsorForm(request.POST if request.method == "POST" else None, sponsor)
 
     if request.method == "POST":
-        _handle_images(request, form)
+        if _color_logo_url := _upload_image("color_logo", request, form):
+            form.color_logo_url.data = _color_logo_url
+        if _white_logo_url := _upload_image("white_logo", request, form):
+            form.white_logo_url.data = _white_logo_url
         if form.validate():
             form.populate_obj(sponsor)
             request.session.flash("Sponsor updated", queue="success")
@@ -179,7 +178,8 @@ def create_sponsor(request):
     form = SponsorForm(request.POST if request.method == "POST" else None)
 
     if request.method == "POST":
-        _handle_images(request, form)
+        form.color_logo_url.data = _upload_image("color_logo", request, form)
+        form.white_logo_url.data = _upload_image("white_logo", request, form)
         if form.validate():
             del form.color_logo
             del form.white_logo