engineering/make_post_sell
1
0
Fork 0
Code Issues 2 Pull requests Projects Releases 2 Packages Wiki Activity Actions

feat: validate_discovery_ring health check + mod-only diagnostic endpoint

Browse source 
Adds validate_discovery_ring(shop) in models/shop.py that returns a
dict diagnosing four ring topology defects:

  - duplicates: IDs appearing more than once in ring (greedy-walk bug)
  - orphans: public products missing from ring (added after reforge)
  - stale: ring IDs no longer public/present (deleted or unlisted
    after reforge — the 'pocket' condition we just patched)
  - length_mismatch: ring_length != public_count

Wired into reforge_discovery_ring_async — anomalies log a warning
after each background reforge, making silent drift visible.

New route /s/{shop_id}/ring/health.json exposes the validator to
shop mods (403 for anon and non-editor users, 404 for missing shop).

Tests across all three layers:
  - Unit (test_models.py, 7 tests): mocked shop.products, each
    anomaly class verified in isolation.
  - Integration (test_integration.py, 4 tests): real shop + products
    + reforge, simulates visibility changes and late additions,
    confirms reforge heals the ring.
  - Functional (test_functional.py, 5 tests): auth required, mod
    ownership enforced, 404 on unknown shop, real-world stale
    detection through the HTTP endpoint.
This commit is contained in:
russell@unturf.com 2026-04-21 18:30:31 -04:00
parent ace97a1ef6
commit 690f945f70
6 changed files with 416 additions and 1 deletions
Download patch file Download diff file Expand all files Collapse all files

52
make_post_sell/models/shop.py
View file

@ -3,6 +3,7 @@ import logging
import threading
import time
import uuid
from collections import Counter
from sqlalchemy import Column, BigInteger, Boolean, Unicode, UnicodeText, func
from sqlalchemy import and_
@ -868,6 +869,45 @@ def reforge_discovery_ring(shop):
return ring
def validate_discovery_ring(shop):
"""Check ring topology health against the shop's public products.
A healthy ring: one entry per visibility==1 product, no duplicates,
no stale IDs, no orphaned products. Any violation surfaces as a
populated list in the returned dict.
Returns:
{
"valid": bool,
"ring_length": int,
"public_count": int,
"duplicates": list[str], # IDs appearing more than once
"orphans": list[str], # public product IDs missing from ring
"stale": list[str], # ring IDs no longer public/present
"length_mismatch": bool, # ring_length != public_count
}
"""
ring = shop.discovery_ring or []
public_ids = {str(p.id) for p in shop.products if p.visibility == 1}
ring_ids = set(ring)
counts = Counter(ring)
duplicates = sorted(pid for pid, c in counts.items() if c > 1)
stale = sorted(ring_ids - public_ids)
orphans = sorted(public_ids - ring_ids)
length_mismatch = len(ring) != len(public_ids)
return {
"valid": not (duplicates or stale or orphans or length_mismatch),
"ring_length": len(ring),
"public_count": len(public_ids),
"duplicates": duplicates,
"orphans": orphans,
"stale": stale,
"length_mismatch": length_mismatch,
}
# Tracks which shops have a background reforge in progress and whether
# another reforge was requested while one was already running (dirty bit).
_reforge_running = {} # shop_id_str -> True while a thread is active
@ -909,6 +949,18 @@ def reforge_discovery_ring_async(shop_id, session_factory):
session.commit()
log.info("Ring reforged for shop %s (%d products)", shop.name, len(ring))
health = validate_discovery_ring(shop)
if not health["valid"]:
log.warning(
"Ring topology anomaly for shop %s: "
"duplicates=%d orphans=%d stale=%d length_mismatch=%s",
shop.name,
len(health["duplicates"]),
len(health["orphans"]),
len(health["stale"]),
health["length_mismatch"],
)
with _reforge_guard:
if _reforge_dirty.pop(shop_id_str, False):
session.expire_all()

1
make_post_sell/routes.py
View file

@ -158,6 +158,7 @@ def includeme(config):
config.add_route("shop_about_slug", "/s/{shop_id}/{slug:.*}/about")
config.add_route("discovery_ring_json", "/s/{shop_id}/ring.json")
config.add_route("discovery_ring_health", "/s/{shop_id}/ring/health.json")
config.add_route("shop_slug", "/s/{shop_id}/{slug:.*}")
# content routes.

110
make_post_sell/tests/test_functional.py
View file

@ -3056,6 +3056,116 @@ class AuthenticatedFunctionalTests(_AuthenticatedBase):
# Should NOT have triggered reforge
mock_async.assert_not_called()
def test_ring_health_requires_login(self):
"""Anonymous request to ring health endpoint is forbidden."""
shop = self._create_shop_helper(
shop_params={**self.shop1_params, "name": "ring-health-anon-shop"}
)
shop_id = str(shop.id)
self.testapp.get("/log-out")
res = self.testapp.get(f"/s/{shop_id}/ring/health.json", status=403)
self.assertIn("Shop editor access required", res.json["error"])
def test_ring_health_requires_mod(self):
"""A logged-in non-owner cannot access another shop's ring health."""
shop = self._create_shop_helper(
shop_params={**self.shop1_params, "name": "ring-health-mod-shop"},
log_out_user=True,
)
shop_id = str(shop.id)
# Log in as a different user who doesn't own this shop
self.log_in_user(self.user2_creds)
res = self.testapp.get(f"/s/{shop_id}/ring/health.json", status=403)
self.assertIn("Shop editor access required", res.json["error"])
def test_ring_health_returns_valid_json_for_mod(self):
"""Shop owner gets a populated health report with healthy ring."""
shop = self._create_shop_helper(
shop_params={**self.shop1_params, "name": "ring-health-ok-shop"}
)
shop_id = str(shop.id)
# Enable watch mode and create 2 products so the ring has content
self.testapp.post(
f"/s/{shop_id}/settings",
{"form_section": "ribbon-settings", "watch_mode": "1", "submit": "Save Settings"},
status=302,
)
self.testapp.post(
f"/p/new?shop_id={shop_id}", self.product1_params
).follow()
self.testapp.post(
f"/p/new?shop_id={shop_id}",
{**self.product1_params, "title": "second product"},
).follow()
# Force a synchronous reforge so the ring is populated deterministically
from ..models.shop import reforge_discovery_ring
self.dbsession.expire(shop)
reforge_discovery_ring(shop)
self.dbsession.flush()
transaction.commit()
res = self.testapp.get(f"/s/{shop_id}/ring/health.json", status=200)
data = res.json
self.assertTrue(data["valid"])
self.assertEqual(data["ring_length"], 2)
self.assertEqual(data["public_count"], 2)
self.assertEqual(data["duplicates"], [])
self.assertEqual(data["orphans"], [])
self.assertEqual(data["stale"], [])
self.assertFalse(data["length_mismatch"])
self.assertEqual(data["shop_id"], shop_id)
def test_ring_health_detects_stale_after_visibility_change(self):
"""Flipping a product to unlisted surfaces in the health report."""
shop = self._create_shop_helper(
shop_params={**self.shop1_params, "name": "ring-health-stale-shop"}
)
shop_id = str(shop.id)
self.testapp.post(
f"/s/{shop_id}/settings",
{"form_section": "ribbon-settings", "watch_mode": "1", "submit": "Save Settings"},
status=302,
)
self.testapp.post(
f"/p/new?shop_id={shop_id}", self.product1_params
).follow()
self.testapp.post(
f"/p/new?shop_id={shop_id}",
{**self.product1_params, "title": "second product"},
).follow()
from ..models.shop import reforge_discovery_ring
self.dbsession.expire(shop)
reforge_discovery_ring(shop)
products = get_all_products(self.dbsession).all()
# Mark one product unlisted — ring still contains it → stale
stale_id = str(products[0].id)
products[0].visibility = 2
self.dbsession.flush()
transaction.commit()
res = self.testapp.get(f"/s/{shop_id}/ring/health.json", status=200)
data = res.json
self.assertFalse(data["valid"])
self.assertIn(stale_id, data["stale"])
def test_ring_health_shop_not_found(self):
"""Unknown shop_id returns 404."""
self._create_shop_helper(
shop_params={**self.shop1_params, "name": "ring-health-404-shop"}
)
res = self.testapp.get(
"/s/00000000-0000-0000-0000-000000000000/ring/health.json",
status=404,
)
self.assertIn("Shop not found", res.json["error"])
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().

106
make_post_sell/tests/test_integration.py
View file

@ -3351,6 +3351,112 @@ class TestAsyncRingReforgeIntegration(DatabaseIntegrationTests):
self.assertEqual(ring[0], public_product_id)
class TestValidateDiscoveryRingIntegration(DatabaseIntegrationTests):
"""End-to-end validator tests against real shop + product rows."""
def _make_shop(self, name="validate-ring-shop"):
user = get_or_create_user_by_email(self.dbsession, "validate@example.com")
self.dbsession.add(user)
self.dbsession.flush()
shop = Shop(name, "555-555-5555", "123 Validate St", "desc")
shop.watch_mode_enabled = True
self.dbsession.add(shop)
self.dbsession.flush()
return shop
def _make_products(self, shop, count, visibility=1):
products = []
for i in range(count):
p = Product(f"Song {i}", f"Desc {i}")
p.shop_id = shop.id
p.visibility = visibility
self.dbsession.add(p)
products.append(p)
self.dbsession.flush()
return products
def test_valid_after_fresh_reforge(self):
"""Immediately after reforge, validator reports healthy ring."""
from ..models.shop import reforge_discovery_ring, validate_discovery_ring
shop = self._make_shop()
self._make_products(shop, 5)
reforge_discovery_ring(shop)
self.dbsession.flush()
health = validate_discovery_ring(shop)
self.assertTrue(health["valid"])
self.assertEqual(health["ring_length"], 5)
self.assertEqual(health["public_count"], 5)
self.assertEqual(health["orphans"], [])
self.assertEqual(health["stale"], [])
self.assertEqual(health["duplicates"], [])
def test_detects_orphan_after_new_product(self):
"""Adding a public product after reforge leaves it orphaned."""
from ..models.shop import reforge_discovery_ring, validate_discovery_ring
shop = self._make_shop(name="orphan-shop")
self._make_products(shop, 3)
reforge_discovery_ring(shop)
self.dbsession.flush()
new_product = Product("Latecomer", "Arrived post-reforge")
new_product.shop_id = shop.id
new_product.visibility = 1
self.dbsession.add(new_product)
self.dbsession.flush()
health = validate_discovery_ring(shop)
self.assertFalse(health["valid"])
self.assertEqual(health["orphans"], [str(new_product.id)])
self.assertTrue(health["length_mismatch"])
def test_detects_stale_after_visibility_change(self):
"""Flipping a product to unlisted leaves a stale entry in the ring."""
from ..models.shop import reforge_discovery_ring, validate_discovery_ring
shop = self._make_shop(name="stale-shop")
products = self._make_products(shop, 4)
reforge_discovery_ring(shop)
self.dbsession.flush()
# Mark one product unlisted — it stays in the ring but isn't public anymore
products[1].visibility = 2
self.dbsession.flush()
health = validate_discovery_ring(shop)
self.assertFalse(health["valid"])
self.assertEqual(health["stale"], [str(products[1].id)])
def test_reforge_clears_stale_and_orphan(self):
"""A fresh reforge after changes restores a healthy ring."""
from ..models.shop import reforge_discovery_ring, validate_discovery_ring
shop = self._make_shop(name="heal-shop")
products = self._make_products(shop, 3)
reforge_discovery_ring(shop)
self.dbsession.flush()
# Add orphan and create stale
new_product = Product("Orphan", "Added later")
new_product.shop_id = shop.id
new_product.visibility = 1
self.dbsession.add(new_product)
products[0].visibility = 0 # now private → stale in ring
self.dbsession.flush()
before = validate_discovery_ring(shop)
self.assertFalse(before["valid"])
reforge_discovery_ring(shop)
self.dbsession.flush()
after = validate_discovery_ring(shop)
self.assertTrue(after["valid"])
class TestKaraokeTrackAclIntegration(DatabaseIntegrationTests):
"""Integration tests for karaoke track ACL behavior across visibility changes."""

122
make_post_sell/tests/test_models.py
View file

@ -2566,6 +2566,128 @@ class TestDiscoveryRing(unittest.TestCase):
self.assertEqual(json.loads(shop.json_discovery_ring), ring)
class TestValidateDiscoveryRing(unittest.TestCase):
"""Test the ring topology validator."""
def _make_shop(self):
return Shop(
"validate-shop",
"555-555-5555",
"123 Ring St",
"Shop for validator tests",
)
def _make_product(self, title, desc, visibility=1):
from ..models.product import Product
p = Product(title, desc)
p.visibility = visibility
p.created_timestamp = 100
return p
def test_valid_ring(self):
"""Ring exactly matches public products → valid."""
from ..models.shop import validate_discovery_ring
shop = self._make_shop()
p1 = self._make_product("A", "alpha")
p2 = self._make_product("B", "beta")
p3 = self._make_product("C", "gamma")
shop.discovery_ring = [str(p1.id), str(p2.id), str(p3.id)]
with mock.patch.object(type(shop), 'products', new_callable=mock.PropertyMock) as mp:
mp.return_value = [p1, p2, p3]
health = validate_discovery_ring(shop)
self.assertTrue(health["valid"])
self.assertEqual(health["ring_length"], 3)
self.assertEqual(health["public_count"], 3)
self.assertEqual(health["duplicates"], [])
self.assertEqual(health["orphans"], [])
self.assertEqual(health["stale"], [])
self.assertFalse(health["length_mismatch"])
def test_empty_shop_empty_ring_is_valid(self):
"""No products + empty ring is still a valid state."""
from ..models.shop import validate_discovery_ring
shop = self._make_shop()
shop.discovery_ring = []
with mock.patch.object(type(shop), 'products', new_callable=mock.PropertyMock) as mp:
mp.return_value = []
health = validate_discovery_ring(shop)
self.assertTrue(health["valid"])
self.assertEqual(health["ring_length"], 0)
self.assertEqual(health["public_count"], 0)
def test_detects_duplicates(self):
"""Same ID appearing twice in the ring is flagged."""
from ..models.shop import validate_discovery_ring
shop = self._make_shop()
p1 = self._make_product("A", "alpha")
p2 = self._make_product("B", "beta")
shop.discovery_ring = [str(p1.id), str(p2.id), str(p1.id)]
with mock.patch.object(type(shop), 'products', new_callable=mock.PropertyMock) as mp:
mp.return_value = [p1, p2]
health = validate_discovery_ring(shop)
self.assertFalse(health["valid"])
self.assertEqual(health["duplicates"], [str(p1.id)])
def test_detects_orphans(self):
"""Public products missing from ring are flagged as orphans."""
from ..models.shop import validate_discovery_ring
shop = self._make_shop()
p1 = self._make_product("A", "alpha")
p2 = self._make_product("B", "beta")
p3 = self._make_product("C", "gamma")
# p3 is public but not in ring (added after reforge)
shop.discovery_ring = [str(p1.id), str(p2.id)]
with mock.patch.object(type(shop), 'products', new_callable=mock.PropertyMock) as mp:
mp.return_value = [p1, p2, p3]
health = validate_discovery_ring(shop)
self.assertFalse(health["valid"])
self.assertEqual(health["orphans"], [str(p3.id)])
self.assertTrue(health["length_mismatch"])
def test_detects_stale(self):
"""Ring IDs whose products became non-public are stale."""
from ..models.shop import validate_discovery_ring
shop = self._make_shop()
p1 = self._make_product("A", "alpha", visibility=1)
p2 = self._make_product("B", "beta", visibility=2) # unlisted now
p3 = self._make_product("C", "gamma", visibility=0) # private now
shop.discovery_ring = [str(p1.id), str(p2.id), str(p3.id)]
with mock.patch.object(type(shop), 'products', new_callable=mock.PropertyMock) as mp:
mp.return_value = [p1, p2, p3]
health = validate_discovery_ring(shop)
self.assertFalse(health["valid"])
self.assertEqual(sorted(health["stale"]), sorted([str(p2.id), str(p3.id)]))
def test_detects_stale_deleted_product(self):
"""Ring IDs that don't exist in shop.products at all are stale."""
from ..models.shop import validate_discovery_ring
shop = self._make_shop()
p1 = self._make_product("A", "alpha")
ghost_id = "deadbeef-dead-beef-dead-beefdeadbeef"
shop.discovery_ring = [str(p1.id), ghost_id]
with mock.patch.object(type(shop), 'products', new_callable=mock.PropertyMock) as mp:
mp.return_value = [p1]
health = validate_discovery_ring(shop)
self.assertFalse(health["valid"])
self.assertIn(ghost_id, health["stale"])
def test_length_mismatch_without_orphans_or_stale(self):
"""Ring with duplicates but all IDs present has length mismatch."""
from ..models.shop import validate_discovery_ring
shop = self._make_shop()
p1 = self._make_product("A", "alpha")
p2 = self._make_product("B", "beta")
# Duplicate of p1 — ring length 3, public_count 2
shop.discovery_ring = [str(p1.id), str(p1.id), str(p2.id)]
with mock.patch.object(type(shop), 'products', new_callable=mock.PropertyMock) as mp:
mp.return_value = [p1, p2]
health = validate_discovery_ring(shop)
self.assertFalse(health["valid"])
self.assertTrue(health["length_mismatch"])
self.assertEqual(health["ring_length"], 3)
self.assertEqual(health["public_count"], 2)
class TestAsyncDiscoveryRing(unittest.TestCase):
"""Test the async ring reforge with dirty bit debounce."""

26
make_post_sell/views/watch.py
View file

@ -3,7 +3,7 @@ import logging
from pyramid.view import view_config
from ..models.product import get_media_type, get_related_products, get_ring_related_products
from ..models.shop import get_shop_by_id
from ..models.shop import get_shop_by_id, validate_discovery_ring
from ..lib.currency import cents_to_dollars
from ..lib.time_funcs import timestamp_to_ago_string, timestamp_to_datetime
@ -309,3 +309,27 @@ def discovery_ring_json(request):
"ring_length": len(ring),
"ring": ring,
}
@view_config(route_name="discovery_ring_health", renderer="json")
def discovery_ring_health(request):
"""Mod-only JSON diagnostic for discovery ring topology.
Returns duplicates, orphans (public products missing from ring),
stale (ring IDs no longer public), and length mismatch flags.
"""
shop_id = request.matchdict.get("shop_id")
shop = get_shop_by_id(request.dbsession, shop_id)
if not shop:
request.response.status_int = 404
return {"error": "Shop not found"}
if not (request.user and request.user.authenticated
and request.user.can_edit_shop(shop)):
request.response.status_int = 403
return {"error": "Shop editor access required"}
health = validate_discovery_ring(shop)
health["shop_id"] = str(shop.id)
health["shop_name"] = shop.name
return health