Lazy cart creation: don't persist session carts until product added
Bots and crawlers were creating empty cart rows on every page visit, bloating the database (531k empty rows, 80% of all carts). Session carts are now kept in-memory until a product is actually added, at which point the view persists them via dbsession.add + flush. - add_session_cart returns in-memory Cart, reuses UUID across requests - authentication.py guards dbsession.delete for transient carts - 17 new tests: unit, integration, and functional coverage
This commit is contained in:
parent
5ee2175a3f
commit
4c1cd11050
6 changed files with 380 additions and 8 deletions
|
|
@ -1 +1 @@
|
|||
d03178b
|
||||
da46586
|
||||
|
|
@ -80,19 +80,30 @@ def includeme(config):
|
|||
return request.session_cart
|
||||
|
||||
def add_session_cart(request):
|
||||
"""Return Cart from session or None."""
|
||||
"""Return Cart from session or an in-memory Cart.
|
||||
|
||||
The cart is NOT persisted to the database until a product is
|
||||
actually added. This prevents bots and crawlers from creating
|
||||
empty cart rows on every page visit.
|
||||
"""
|
||||
import uuid as uuid_mod
|
||||
|
||||
cart_id = request.session.get("active_cart_id", None)
|
||||
if cart_id is not None:
|
||||
cart = get_cart_by_id(request.dbsession, cart_id)
|
||||
if cart is not None:
|
||||
return cart
|
||||
# if no active cart in session, create one.
|
||||
|
||||
# Return an in-memory cart (not yet in the DB).
|
||||
# Reuse the session cart_id when one exists so the UUID stays
|
||||
# stable across requests (e.g. /cart redirects to /cart/{id}).
|
||||
cart = Cart()
|
||||
cart.shop = request.shop
|
||||
cart.active = True
|
||||
request.session["active_cart_id"] = cart.uuid_str
|
||||
request.dbsession.add(cart)
|
||||
request.dbsession.flush()
|
||||
if cart_id is not None:
|
||||
cart.id = uuid_mod.UUID(cart_id)
|
||||
else:
|
||||
request.session["active_cart_id"] = cart.uuid_str
|
||||
return cart
|
||||
|
||||
def add_shop_location(request):
|
||||
|
|
|
|||
|
|
@ -2634,3 +2634,42 @@ class AuthenticatedFunctionalTests(FunctionalTests):
|
|||
self.assertIn(b'type="application/atom+xml"', res.body)
|
||||
self.assertIn(b"/rss.xml", res.body)
|
||||
self.assertIn(b"/atom.xml", res.body)
|
||||
|
||||
|
||||
class LazyCartFunctionalTests(FunctionalTests):
|
||||
"""Functional tests for lazy cart creation.
|
||||
|
||||
Session carts should NOT be persisted to the database until a product
|
||||
is actually added. Anonymous browsing (bots, crawlers) should create
|
||||
zero cart rows.
|
||||
"""
|
||||
|
||||
def test_anonymous_browse_creates_no_cart_rows(self):
|
||||
"""Browsing pages as an anonymous user should not create any cart rows."""
|
||||
# Count carts before
|
||||
carts_before = get_all_carts(self.dbsession).count()
|
||||
|
||||
# Browse several pages as anonymous user
|
||||
self.testapp.get("/", status=200)
|
||||
self.testapp.get("/join-or-log-in", status=200)
|
||||
self.testapp.get("/", status=200)
|
||||
|
||||
# No new carts should have been created
|
||||
carts_after = get_all_carts(self.dbsession).count()
|
||||
self.assertEqual(carts_before, carts_after)
|
||||
|
||||
def test_anonymous_browse_shows_empty_cart_in_nav(self):
|
||||
"""Nav bar should show $0.00 (0) for anonymous users with lazy carts."""
|
||||
res = self.testapp.get("/", status=200)
|
||||
self.assertIn(b"Cart $0.00 (0)", res.body)
|
||||
|
||||
def test_cart_page_works_with_lazy_cart(self):
|
||||
"""Visiting /cart should work even when the session cart is in-memory."""
|
||||
# Browse the home page first (creates in-memory session cart)
|
||||
self.testapp.get("/", status=200)
|
||||
|
||||
# Visit /cart — should redirect to /cart/{id} and render without error
|
||||
res = self.testapp.get("/cart", status=302)
|
||||
res = res.follow()
|
||||
# Should show an empty cart page (status 200)
|
||||
self.assertEqual(200, res.status_int)
|
||||
|
|
|
|||
|
|
@ -3044,3 +3044,116 @@ class TestAdyenIntegration(DatabaseIntegrationTests):
|
|||
self.assertEqual(invoice_paypal_adyen.payment_method, "paypal")
|
||||
|
||||
transaction.commit()
|
||||
|
||||
|
||||
class TestLazyCartCreationIntegration(DatabaseIntegrationTests):
|
||||
"""Integration tests for lazy cart creation.
|
||||
|
||||
Session carts should NOT be persisted to the database until a product
|
||||
is actually added. This prevents bots from creating empty cart rows.
|
||||
"""
|
||||
|
||||
def test_inmemory_cart_not_in_database(self):
|
||||
"""A transient Cart (never added to session) should not appear in DB."""
|
||||
cart = Cart()
|
||||
cart.active = True
|
||||
|
||||
# Cart has a valid id but is not in DB
|
||||
self.assertIsNotNone(cart.id)
|
||||
found = self.dbsession.query(Cart).filter(Cart.id == cart.id).first()
|
||||
self.assertIsNone(found)
|
||||
|
||||
def test_inmemory_cart_persists_after_dbsession_add(self):
|
||||
"""When dbsession.add + flush is called, the cart appears in DB."""
|
||||
cart = Cart()
|
||||
cart.active = True
|
||||
self.assertIsNone(cart.dbsession)
|
||||
|
||||
self.dbsession.add(cart)
|
||||
self.dbsession.flush()
|
||||
|
||||
# Now it should be found
|
||||
self.assertIsNotNone(cart.dbsession)
|
||||
found = self.dbsession.query(Cart).filter(Cart.id == cart.id).first()
|
||||
self.assertIsNotNone(found)
|
||||
self.assertEqual(found.id, cart.id)
|
||||
|
||||
def test_add_product_then_persist(self):
|
||||
"""Simulate the lazy cart flow: add product, then persist."""
|
||||
user = get_or_create_user_by_email(self.dbsession, "lazy@example.com")
|
||||
self.dbsession.add(user)
|
||||
|
||||
shop = Shop(
|
||||
name="Lazy Cart Shop",
|
||||
phone_number="555-555-5555",
|
||||
billing_address="123 Test St",
|
||||
description="A test shop",
|
||||
)
|
||||
shop.stripe_public_api_key = "pk_test_123"
|
||||
shop.stripe_secret_api_key = "sk_test_123"
|
||||
shop.domain_name = "lazy.localhost"
|
||||
self.dbsession.add(shop)
|
||||
|
||||
product = Product(title="Digital Download", description="Test product")
|
||||
product.shop_id = shop.id
|
||||
product.price_in_cents = 500
|
||||
product.is_physical = False
|
||||
self.dbsession.add(product)
|
||||
self.dbsession.flush()
|
||||
|
||||
# Create in-memory cart (simulates lazy session cart)
|
||||
cart = Cart()
|
||||
cart.shop = shop
|
||||
cart.active = True
|
||||
self.assertIsNone(cart.dbsession)
|
||||
|
||||
# Count carts before
|
||||
cart_count_before = self.dbsession.query(Cart).count()
|
||||
|
||||
# Add product (this is what triggers persistence in the view)
|
||||
cart.add_product(product)
|
||||
self.dbsession.add(cart)
|
||||
self.dbsession.flush()
|
||||
|
||||
# Now the cart should be in the DB
|
||||
cart_count_after = self.dbsession.query(Cart).count()
|
||||
self.assertEqual(cart_count_after, cart_count_before + 1)
|
||||
|
||||
found = self.dbsession.query(Cart).filter(Cart.id == cart.id).first()
|
||||
self.assertIsNotNone(found)
|
||||
self.assertEqual(found.count, 1)
|
||||
|
||||
def test_login_merge_skips_delete_for_transient_cart(self):
|
||||
"""The login merge should not crash when session_cart is transient.
|
||||
|
||||
authentication.py checks session_cart.dbsession before calling
|
||||
dbsession.delete(). A transient cart has dbsession=None.
|
||||
"""
|
||||
# Create a transient cart (simulates unpersisted session cart)
|
||||
session_cart = Cart()
|
||||
session_cart.active = True
|
||||
|
||||
# Verify the guard condition works
|
||||
self.assertIsNone(session_cart.dbsession)
|
||||
|
||||
# This is the pattern from authentication.py:
|
||||
# if request.session_cart.dbsession is not None:
|
||||
# request.dbsession.delete(request.session_cart)
|
||||
# Should NOT attempt to delete
|
||||
if session_cart.dbsession is not None:
|
||||
self.fail("Transient cart should have dbsession=None")
|
||||
|
||||
def test_inmemory_cart_reuse_uuid(self):
|
||||
"""Test recreating a Cart with a specific UUID (lazy cart reuse)."""
|
||||
import uuid as uuid_mod
|
||||
|
||||
# First request: create in-memory cart, store UUID in "session"
|
||||
cart1 = Cart()
|
||||
stored_uuid = cart1.uuid_str
|
||||
|
||||
# Second request: recreate cart with same UUID
|
||||
cart2 = Cart()
|
||||
cart2.id = uuid_mod.UUID(stored_uuid)
|
||||
|
||||
self.assertEqual(cart2.uuid_str, stored_uuid)
|
||||
self.assertEqual(str(cart2.id), str(cart1.id))
|
||||
|
|
|
|||
|
|
@ -555,6 +555,84 @@ class TestCart(unittest.TestCase):
|
|||
# Should use international shipping rate
|
||||
self.assertEqual(cart.handling_cost_in_cents, 1500)
|
||||
|
||||
def test_transient_cart_count_is_zero(self):
|
||||
"""Test that an in-memory Cart (not added to DB) has count=0."""
|
||||
from make_post_sell.models.cart import Cart
|
||||
|
||||
cart = Cart()
|
||||
self.assertEqual(cart.count, 0)
|
||||
|
||||
def test_transient_cart_is_empty(self):
|
||||
"""Test that an in-memory Cart reports is_empty=True."""
|
||||
from make_post_sell.models.cart import Cart
|
||||
|
||||
cart = Cart()
|
||||
self.assertTrue(cart.is_empty)
|
||||
|
||||
def test_transient_cart_dbsession_is_none(self):
|
||||
"""Test that a transient Cart (never added to a DB session) has no dbsession.
|
||||
|
||||
This is critical for lazy cart creation: the login flow checks
|
||||
session_cart.dbsession to decide whether to call dbsession.delete().
|
||||
"""
|
||||
from make_post_sell.models.cart import Cart
|
||||
|
||||
cart = Cart()
|
||||
self.assertIsNone(cart.dbsession)
|
||||
|
||||
def test_transient_cart_has_valid_uuid(self):
|
||||
"""Test that a transient Cart has a valid UUID id and uuid_str."""
|
||||
from make_post_sell.models.cart import Cart
|
||||
|
||||
cart = Cart()
|
||||
self.assertIsNotNone(cart.id)
|
||||
self.assertIsInstance(cart.uuid_str, str)
|
||||
self.assertGreater(len(cart.uuid_str), 0)
|
||||
|
||||
def test_transient_cart_total_is_zero(self):
|
||||
"""Test that an in-memory empty Cart returns total=0.00.
|
||||
|
||||
The nav bar accesses request.active_cart.total on every page.
|
||||
For lazy carts, this must work without a DB session.
|
||||
"""
|
||||
from make_post_sell.models.cart import Cart
|
||||
|
||||
cart = Cart()
|
||||
|
||||
# Mock the products and coupons to avoid DB access
|
||||
with mock.patch.object(
|
||||
type(cart), "products", new_callable=mock.PropertyMock
|
||||
) as mock_products:
|
||||
mock_products.return_value = {}
|
||||
|
||||
with mock.patch.object(
|
||||
type(cart), "coupons", new_callable=mock.PropertyMock
|
||||
) as mock_coupons:
|
||||
mock_coupons.return_value = []
|
||||
|
||||
self.assertEqual(cart.total, 0.00)
|
||||
self.assertEqual(cart.count, 0)
|
||||
|
||||
def test_transient_cart_uuid_reusable(self):
|
||||
"""Test that a Cart's id can be overwritten to reuse a UUID.
|
||||
|
||||
Lazy cart creation reuses the session UUID when recreating
|
||||
an in-memory cart across requests.
|
||||
"""
|
||||
from make_post_sell.models.cart import Cart
|
||||
import uuid as uuid_mod
|
||||
|
||||
cart = Cart()
|
||||
original_id = cart.id
|
||||
|
||||
# Overwrite with a specific UUID (simulates lazy cart reuse)
|
||||
new_uuid = uuid_mod.UUID("75b1de48-414b-11e7-aecf-9c4e369c7158")
|
||||
cart.id = new_uuid
|
||||
|
||||
self.assertEqual(cart.id, new_uuid)
|
||||
self.assertNotEqual(cart.id, original_id)
|
||||
self.assertEqual(cart.uuid_str, str(new_uuid))
|
||||
|
||||
def test_cart_total_property_not_discounted(self):
|
||||
"""Test Cart.total property when cart is not discounted (line 347)."""
|
||||
from make_post_sell.models.cart import Cart
|
||||
|
|
@ -577,6 +655,136 @@ class TestCart(unittest.TestCase):
|
|||
mock_total_price.assert_called_once()
|
||||
|
||||
|
||||
class TestLazySessionCart(unittest.TestCase):
|
||||
"""Test the lazy cart creation logic in add_session_cart.
|
||||
|
||||
Session carts are NOT persisted to the database until a product is
|
||||
actually added. This prevents bots and crawlers from creating
|
||||
empty cart rows on every page visit.
|
||||
"""
|
||||
|
||||
def _make_mock_request(self, session_data=None, shop=None, db_cart=None):
|
||||
"""Build a mock Pyramid request for testing add_session_cart."""
|
||||
request = mock.MagicMock()
|
||||
request.session = dict(session_data or {})
|
||||
request.shop = shop
|
||||
|
||||
mock_dbsession = mock.MagicMock()
|
||||
request.dbsession = mock_dbsession
|
||||
|
||||
# Make get_cart_by_id return db_cart when called
|
||||
return request, db_cart
|
||||
|
||||
def test_returns_persisted_cart_when_found_in_db(self):
|
||||
"""When session has a cart_id and it exists in the DB, return it."""
|
||||
from make_post_sell.models.cart import Cart
|
||||
|
||||
persisted_cart = Cart()
|
||||
request, _ = self._make_mock_request(
|
||||
session_data={"active_cart_id": persisted_cart.uuid_str},
|
||||
)
|
||||
|
||||
with mock.patch(
|
||||
"make_post_sell.request_methods.get_cart_by_id",
|
||||
return_value=persisted_cart,
|
||||
) as mock_get:
|
||||
# Import and call the function under test
|
||||
from make_post_sell.request_methods import includeme
|
||||
|
||||
# We need to test the inner function directly. Extract it by
|
||||
# calling includeme and capturing the registered function.
|
||||
captured = {}
|
||||
|
||||
def fake_add_request_method(func, name, **kwargs):
|
||||
captured[name] = func
|
||||
|
||||
config = mock.MagicMock()
|
||||
config.get_settings.return_value = {}
|
||||
config.add_request_method = fake_add_request_method
|
||||
includeme(config)
|
||||
|
||||
add_session_cart = captured["session_cart"]
|
||||
result = add_session_cart(request)
|
||||
|
||||
mock_get.assert_called_once_with(
|
||||
request.dbsession, persisted_cart.uuid_str
|
||||
)
|
||||
self.assertEqual(result, persisted_cart)
|
||||
|
||||
def test_creates_inmemory_cart_when_no_session_id(self):
|
||||
"""When session has no cart_id, create an in-memory Cart and store UUID."""
|
||||
request, _ = self._make_mock_request()
|
||||
|
||||
with mock.patch(
|
||||
"make_post_sell.request_methods.get_cart_by_id",
|
||||
return_value=None,
|
||||
):
|
||||
captured = {}
|
||||
|
||||
def fake_add_request_method(func, name, **kwargs):
|
||||
captured[name] = func
|
||||
|
||||
config = mock.MagicMock()
|
||||
config.get_settings.return_value = {}
|
||||
config.add_request_method = fake_add_request_method
|
||||
|
||||
from make_post_sell.request_methods import includeme
|
||||
|
||||
includeme(config)
|
||||
add_session_cart = captured["session_cart"]
|
||||
result = add_session_cart(request)
|
||||
|
||||
# Should have stored the cart UUID in the session
|
||||
self.assertIn("active_cart_id", request.session)
|
||||
self.assertEqual(request.session["active_cart_id"], result.uuid_str)
|
||||
|
||||
# Cart should NOT be added to the DB session
|
||||
request.dbsession.add.assert_not_called()
|
||||
request.dbsession.flush.assert_not_called()
|
||||
|
||||
# Cart should be usable (count=0, empty)
|
||||
self.assertEqual(result.count, 0)
|
||||
self.assertTrue(result.is_empty)
|
||||
self.assertTrue(result.active)
|
||||
|
||||
def test_reuses_uuid_from_session_when_cart_not_in_db(self):
|
||||
"""When session has a cart_id but DB returns None, reuse the same UUID."""
|
||||
import uuid as uuid_mod
|
||||
|
||||
stale_uuid = str(uuid_mod.uuid4())
|
||||
request, _ = self._make_mock_request(
|
||||
session_data={"active_cart_id": stale_uuid},
|
||||
)
|
||||
|
||||
with mock.patch(
|
||||
"make_post_sell.request_methods.get_cart_by_id",
|
||||
return_value=None,
|
||||
):
|
||||
captured = {}
|
||||
|
||||
def fake_add_request_method(func, name, **kwargs):
|
||||
captured[name] = func
|
||||
|
||||
config = mock.MagicMock()
|
||||
config.get_settings.return_value = {}
|
||||
config.add_request_method = fake_add_request_method
|
||||
|
||||
from make_post_sell.request_methods import includeme
|
||||
|
||||
includeme(config)
|
||||
add_session_cart = captured["session_cart"]
|
||||
result = add_session_cart(request)
|
||||
|
||||
# UUID should match what was in the session
|
||||
self.assertEqual(result.uuid_str, stale_uuid)
|
||||
|
||||
# Session should NOT be overwritten
|
||||
self.assertEqual(request.session["active_cart_id"], stale_uuid)
|
||||
|
||||
# Cart should NOT be added to the DB
|
||||
request.dbsession.add.assert_not_called()
|
||||
|
||||
|
||||
class TestCoupon(unittest.TestCase):
|
||||
@mock.patch("make_post_sell.models.user.is_user_name_available", mock_always_true)
|
||||
def setUp(self):
|
||||
|
|
|
|||
|
|
@ -101,9 +101,10 @@ def verification_challenge(request):
|
|||
request.session["authenticated_user_id"] = str(user.id)
|
||||
request.session.flash(msg)
|
||||
|
||||
# Merge carts
|
||||
# Merge carts (session_cart may be in-memory if no items were added)
|
||||
request.active_cart.merge_in_cart(request.session_cart)
|
||||
request.dbsession.delete(request.session_cart)
|
||||
if request.session_cart.dbsession is not None:
|
||||
request.dbsession.delete(request.session_cart)
|
||||
request.session["active_cart_id"] = str(request.active_cart.id)
|
||||
request.dbsession.add(user)
|
||||
request.dbsession.add(request.active_cart)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue