fix: update_s3_acls now handles karaoke tracks + add tests
update_s3_acls silently skipped instrumentals/vocals because no s3_key resolution existed for those file keys. Added generic fallback to construct s3_path/file_key for any unhandled key. Added 11 tests: unit (ACL parity across visibility levels, file_keys membership, update_s3_acls call count), integration (visibility change propagates to karaoke ACLs), functional (watch JSON and content page karaoke URL embedding).
This commit is contained in:
parent
f0ca717ff9
commit
a8676025e4
4 changed files with 231 additions and 0 deletions
|
|
@ -565,6 +565,8 @@ class Product(RBase, Base):
|
|||
s3_key = self.s3_key_preview
|
||||
elif s3_key is None and file_key.startswith("thumbnail"):
|
||||
s3_key = self.s3_key_thumbnail(file_key)
|
||||
elif s3_key is None:
|
||||
s3_key = f"{self.s3_path}/{file_key}"
|
||||
|
||||
if s3_key:
|
||||
try:
|
||||
|
|
|
|||
|
|
@ -3050,6 +3050,98 @@ class AuthenticatedFunctionalTests(FunctionalTests):
|
|||
# Should NOT have triggered reforge
|
||||
mock_async.assert_not_called()
|
||||
|
||||
def _create_content_with_metadata(self, shop_name, title, description, file_metadata):
|
||||
"""Helper: create shop + watch mode + content product with given file_metadata.
|
||||
Returns (product_id_str, product_slug) — safe to use after transaction.commit().
|
||||
"""
|
||||
import json
|
||||
|
||||
shop = self._create_shop_helper(
|
||||
shop_params={**self.shop1_params, "name": shop_name}
|
||||
)
|
||||
self.testapp.post(
|
||||
f"/s/{shop.id}/settings",
|
||||
{"form_section": "ribbon-settings", "watch_mode": "1", "submit": "Save Settings"},
|
||||
status=302,
|
||||
)
|
||||
res = self.testapp.post(
|
||||
"/c/new",
|
||||
{"title": title, "description": description, "submit": True},
|
||||
)
|
||||
if res.status_int == 302:
|
||||
res.follow()
|
||||
|
||||
products = get_all_products(self.dbsession).all()
|
||||
product = products[0]
|
||||
product_id = str(product.id)
|
||||
product_slug = product.slug
|
||||
|
||||
product._file_metadata = file_metadata
|
||||
product.json_file_metadata = json.dumps(file_metadata)
|
||||
product.visibility = 1
|
||||
self.dbsession.flush()
|
||||
transaction.commit()
|
||||
|
||||
return product_id, product_slug
|
||||
|
||||
def test_watch_json_includes_karaoke_urls(self):
|
||||
"""Watch JSON returns instrumentals/vocals URLs for products with karaoke tracks."""
|
||||
product_id, _ = self._create_content_with_metadata(
|
||||
"karaoke-watch-shop", "Karaoke Song", "Test karaoke",
|
||||
{
|
||||
"extensions": {"product": "mp3", "thumbnail1": "jpg", "instrumentals": "wav", "vocals": "wav"},
|
||||
"file_bytes": {"product": 5000000, "instrumentals": 8000000, "vocals": 8000000},
|
||||
},
|
||||
)
|
||||
|
||||
res = self.testapp.get(f"/watch/{product_id}/json", status=200)
|
||||
data = res.json
|
||||
self.assertIn("instrumentals_url", data)
|
||||
self.assertIn("vocals_url", data)
|
||||
self.assertIsNotNone(data["instrumentals_url"])
|
||||
self.assertIsNotNone(data["vocals_url"])
|
||||
|
||||
def test_watch_json_no_karaoke_without_tracks(self):
|
||||
"""Watch JSON returns null karaoke URLs when product has no karaoke tracks."""
|
||||
product_id, _ = self._create_content_with_metadata(
|
||||
"no-karaoke-watch-shop", "Regular Song", "No karaoke",
|
||||
{"extensions": {"product": "mp3"}, "file_bytes": {"product": 5000000}},
|
||||
)
|
||||
|
||||
res = self.testapp.get(f"/watch/{product_id}/json", status=200)
|
||||
data = res.json
|
||||
self.assertIn("instrumentals_url", data)
|
||||
self.assertIn("vocals_url", data)
|
||||
self.assertIsNone(data["instrumentals_url"])
|
||||
self.assertIsNone(data["vocals_url"])
|
||||
|
||||
def test_content_page_embeds_karaoke_data_attrs(self):
|
||||
"""Content page embeds karaoke URLs as data attributes for watch mode."""
|
||||
product_id, product_slug = self._create_content_with_metadata(
|
||||
"karaoke-content-shop", "Karaoke Content", "Has vocal isolation",
|
||||
{
|
||||
"extensions": {"product": "mp3", "thumbnail1": "jpg", "instrumentals": "wav", "vocals": "wav"},
|
||||
"file_bytes": {"product": 5000000, "instrumentals": 8000000, "vocals": 8000000},
|
||||
},
|
||||
)
|
||||
|
||||
res = self.testapp.get(f"/c/{product_id}/{product_slug}", status=200)
|
||||
body = res.body.decode()
|
||||
self.assertIn("data-instrumentals-url", body)
|
||||
self.assertIn("data-vocals-url", body)
|
||||
|
||||
def test_content_page_no_karaoke_attrs_without_tracks(self):
|
||||
"""Content page omits karaoke data attributes when no tracks exist."""
|
||||
product_id, product_slug = self._create_content_with_metadata(
|
||||
"no-karaoke-content-shop", "Plain Content", "No karaoke here",
|
||||
{"extensions": {"product": "mp3"}, "file_bytes": {"product": 5000000}},
|
||||
)
|
||||
|
||||
res = self.testapp.get(f"/c/{product_id}/{product_slug}", status=200)
|
||||
body = res.body.decode()
|
||||
self.assertNotIn("data-instrumentals-url", body)
|
||||
self.assertNotIn("data-vocals-url", body)
|
||||
|
||||
def test_rss_autodiscovery_links(self):
|
||||
"""Test that RSS and Atom autodiscovery links are present in shop pages."""
|
||||
shop = self._create_shop_helper(
|
||||
|
|
|
|||
|
|
@ -3348,3 +3348,77 @@ class TestAsyncRingReforgeIntegration(DatabaseIntegrationTests):
|
|||
|
||||
self.assertEqual(len(ring), 1)
|
||||
self.assertEqual(ring[0], public_product_id)
|
||||
|
||||
|
||||
class TestKaraokeTrackAclIntegration(DatabaseIntegrationTests):
|
||||
"""Integration tests for karaoke track ACL behavior across visibility changes."""
|
||||
|
||||
def _make_shop_and_product(self):
|
||||
from ..models.shop import Shop
|
||||
from ..models.product import Product
|
||||
|
||||
shop = Shop("karaoke-acl-shop", "555-555-5555", "123 Test St", "desc")
|
||||
self.dbsession.add(shop)
|
||||
self.dbsession.flush()
|
||||
|
||||
product = Product("Karaoke Test Track", "A test audio product")
|
||||
product.shop = shop
|
||||
product.shop_id = shop.id
|
||||
product.visibility = 1 # public
|
||||
product._file_metadata = {
|
||||
"extensions": {
|
||||
"product": "mp3",
|
||||
"thumbnail1": "jpg",
|
||||
"instrumentals": "wav",
|
||||
"vocals": "wav",
|
||||
},
|
||||
"file_bytes": {
|
||||
"product": 5000000,
|
||||
"instrumentals": 8000000,
|
||||
"vocals": 8000000,
|
||||
},
|
||||
}
|
||||
self.dbsession.add(product)
|
||||
self.dbsession.flush()
|
||||
return shop, product
|
||||
|
||||
def test_visibility_change_updates_karaoke_acls(self):
|
||||
"""Changing visibility calls put_object_acl for karaoke tracks."""
|
||||
from unittest import mock
|
||||
|
||||
shop, product = self._make_shop_and_product()
|
||||
mock_s3 = mock.Mock()
|
||||
|
||||
# Public → private should update ACLs for all files including karaoke
|
||||
product.set_visibility(0, mock_s3, "test-bucket")
|
||||
|
||||
called_keys = [
|
||||
call.kwargs.get("Key") or call[1].get("Key")
|
||||
for call in mock_s3.put_object_acl.call_args_list
|
||||
]
|
||||
s3_path = f"{shop.id}/{product.id}"
|
||||
self.assertIn(f"{s3_path}/instrumentals", called_keys)
|
||||
self.assertIn(f"{s3_path}/vocals", called_keys)
|
||||
|
||||
# All should be private now
|
||||
for call in mock_s3.put_object_acl.call_args_list:
|
||||
acl = call.kwargs.get("ACL") or call[1].get("ACL")
|
||||
self.assertEqual(acl, "private")
|
||||
|
||||
def test_karaoke_acl_matches_product_file_across_visibility(self):
|
||||
"""Karaoke tracks always get the same ACL as the product file."""
|
||||
shop, product = self._make_shop_and_product()
|
||||
|
||||
for vis in (0, 1, 2):
|
||||
product.visibility = vis
|
||||
product_acl = product.get_s3_acl_for_file_key("product")
|
||||
self.assertEqual(
|
||||
product.get_s3_acl_for_file_key("instrumentals"),
|
||||
product_acl,
|
||||
f"instrumentals ACL mismatch at visibility={vis}",
|
||||
)
|
||||
self.assertEqual(
|
||||
product.get_s3_acl_for_file_key("vocals"),
|
||||
product_acl,
|
||||
f"vocals ACL mismatch at visibility={vis}",
|
||||
)
|
||||
|
|
|
|||
|
|
@ -994,6 +994,69 @@ class TestProductS3Security(unittest.TestCase):
|
|||
self.assertFalse(self.product.is_unlisted)
|
||||
self.assertTrue(self.product.is_private)
|
||||
|
||||
def test_karaoke_tracks_in_file_keys(self):
|
||||
"""Karaoke tracks (instrumentals, vocals) are registered in file_keys."""
|
||||
self.assertIn("instrumentals", self.product.file_keys)
|
||||
self.assertIn("vocals", self.product.file_keys)
|
||||
|
||||
def test_karaoke_acl_public_product(self):
|
||||
"""Karaoke tracks follow same ACL as product file for public products."""
|
||||
self.product.visibility = 1
|
||||
self.assertEqual(
|
||||
self.product.get_s3_acl_for_file_key("instrumentals"),
|
||||
self.product.get_s3_acl_for_file_key("product"),
|
||||
)
|
||||
self.assertEqual(
|
||||
self.product.get_s3_acl_for_file_key("vocals"),
|
||||
self.product.get_s3_acl_for_file_key("product"),
|
||||
)
|
||||
|
||||
def test_karaoke_acl_private_product(self):
|
||||
"""Karaoke tracks are private when product is private."""
|
||||
self.product.visibility = 0
|
||||
self.assertEqual(self.product.get_s3_acl_for_file_key("instrumentals"), "private")
|
||||
self.assertEqual(self.product.get_s3_acl_for_file_key("vocals"), "private")
|
||||
|
||||
def test_karaoke_acl_unlisted_product(self):
|
||||
"""Karaoke tracks follow same ACL as product file for unlisted products."""
|
||||
self.product.visibility = 2
|
||||
self.assertEqual(
|
||||
self.product.get_s3_acl_for_file_key("instrumentals"),
|
||||
self.product.get_s3_acl_for_file_key("product"),
|
||||
)
|
||||
self.assertEqual(
|
||||
self.product.get_s3_acl_for_file_key("vocals"),
|
||||
self.product.get_s3_acl_for_file_key("product"),
|
||||
)
|
||||
|
||||
@mock.patch("builtins.print")
|
||||
def test_update_s3_acls_includes_karaoke_tracks(self, mock_print):
|
||||
"""update_s3_acls sets ACL on karaoke tracks when they exist in extensions."""
|
||||
self.product._file_metadata = {
|
||||
"extensions": {
|
||||
"product": "mp3",
|
||||
"thumbnail1": "jpg",
|
||||
"instrumentals": "wav",
|
||||
"vocals": "wav",
|
||||
}
|
||||
}
|
||||
self.product.visibility = 1
|
||||
|
||||
mock_s3 = mock.Mock()
|
||||
self.product.update_s3_acls(mock_s3, "test-bucket")
|
||||
|
||||
# Should call put_object_acl for all 4 file keys in extensions
|
||||
self.assertEqual(mock_s3.put_object_acl.call_count, 4)
|
||||
|
||||
# Verify karaoke tracks were included by checking the Key arguments
|
||||
called_keys = [
|
||||
call.kwargs.get("Key") or call[1].get("Key")
|
||||
for call in mock_s3.put_object_acl.call_args_list
|
||||
]
|
||||
s3_path = self.product.s3_path
|
||||
self.assertIn(f"{s3_path}/instrumentals", called_keys)
|
||||
self.assertIn(f"{s3_path}/vocals", called_keys)
|
||||
|
||||
|
||||
class TestMetaFunctions(unittest.TestCase):
|
||||
"""Test meta.py utility functions for base64 and UUID handling."""
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue