-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Store cart in cookie instead of HttpSession #13
Conversation
Motivation: Effort to reduce server memory usage
|
||
@Controller | ||
@RequestMapping("/cart") | ||
@SuppressWarnings("unchecked") | ||
public class CartController { | ||
|
||
public static final class Carts { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Should not be in Controller.
public static final class
=> should be a top class in its own file
@@ -1,19 +1,21 @@ | |||
package io.gatling.demostore; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First line of first changed file: only to have a closable thread.
I think we should change the session management instead of a workaround like that.
The cart being in the session is a normal usage of the session.
The session being stored in the server is not.
Either an accessible cache (ie memcache or redis) or using session from cookie (like Play does)
First quick search on the topic:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
||
} else { | ||
HashMap<Integer, Cart> cart = (HashMap<Integer, Cart>) session.getAttribute("cart"); | ||
cart = Carts.fromCookieValue(cartCookie); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't fromCookieValue
to manage the case that cartCookie
is null
?
if (cart.containsKey(id)) { | ||
int qty = cart.get(id).getQuantity(); | ||
cart.put(id, new Cart(id, product.getName(), product.getPrice(), ++qty, product.getImage())); | ||
cart.put(id, new CartEntry(id, product.getName(), product.getPrice(), ++qty, product.getImage())); | ||
} else { | ||
cart.put(id, new Cart(id, product.getName(), product.getPrice(), 1, product.getImage())); | ||
session.setAttribute("cart", cart); | ||
cart.put(id, new CartEntry(id, product.getName(), product.getPrice(), 1, product.getImage())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (cart.containsKey(id)) { | |
int qty = cart.get(id).getQuantity(); | |
cart.put(id, new Cart(id, product.getName(), product.getPrice(), ++qty, product.getImage())); | |
cart.put(id, new CartEntry(id, product.getName(), product.getPrice(), ++qty, product.getImage())); | |
} else { | |
cart.put(id, new Cart(id, product.getName(), product.getPrice(), 1, product.getImage())); | |
session.setAttribute("cart", cart); | |
cart.put(id, new CartEntry(id, product.getName(), product.getPrice(), 1, product.getImage())); | |
int qty = cart.containsKey(id) ? cart.get(id).getQuantity() : 0; | |
cart.put(id, new CartEntry(id, product.getName(), product.getPrice(), qty + 1, product.getImage())); |
private static void updateCartCookie(HttpServletResponse response, Map<Integer, CartEntry> cart) throws IOException { | ||
Cookie cookie = new Cookie(Carts.COOKIE_NAME, Carts.toCookieValue(cart)); | ||
cookie.setPath("/"); | ||
response.addCookie(cookie); | ||
} | ||
|
||
private static void removeCartCookie(HttpServletResponse response) { | ||
Cookie cookie = new Cookie(Carts.COOKIE_NAME, ""); | ||
cookie.setMaxAge(0); | ||
cookie.setPath("/"); | ||
response.addCookie(cookie); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the Carts
inner class?
|
||
public static Map<Integer, CartEntry> fromCookieValue(String cookieValue) throws IOException { | ||
if (cookieValue == null) { | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return null; | |
return new HashMap<>(); |
|
||
private static TypeReference<Map<Integer, CartEntry>> CART_TYPE = new TypeReference<Map<Integer, CartEntry>>() {}; | ||
|
||
public static Map<Integer, CartEntry> fromCookieValue(String cookieValue) throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public static Map<Integer, CartEntry> fromCookieValue(String cookieValue) throws IOException { | |
public static Map<Integer, CartEntry> fromCookieValue(String cookieValue) { |
return null; | ||
} | ||
|
||
return JACKSON.readValue(Base64.getDecoder().decode(cookieValue), CART_TYPE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return JACKSON.readValue(Base64.getDecoder().decode(cookieValue), CART_TYPE); | |
try { | |
return JACKSON.readValue(Base64.getDecoder().decode(cookieValue), CART_TYPE); | |
} catch (IOException e) { | |
return new HashMap<>(); | |
} |
Motivation:
Effort to reduce server memory usage