Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions java_cart_codebase/CachedCartService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package com.ecommerce.service;

import com.ecommerce.model.Product;
import com.ecommerce.repository.ProductRepository;
import org.springframework.cache.annotation.Cacheable;
import org.springframework.stereotype.Service;

import java.math.BigDecimal;
import java.util.List;

@Service
public class CachedCartService {

private final ProductRepository productRepository;

public CachedCartService(ProductRepository productRepository) {
this.productRepository = productRepository;
}

@Cacheable(value = "productPrices", key = "#productId")
public BigDecimal getProductPrice(Long productId) {
try {
Thread.sleep(100);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}

Product product = productRepository.findById(productId).orElse(null);
Comment on lines +21 to +28

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove artificial delay in cached method

The getProductPrice method includes a Thread.sleep(100) which introduces unnecessary 100ms delays in production code. This can materially degrade performance, particularly when calculateBulkCartTotal calls it multiple times for bulk operations, even though caching mitigates some impact on cache hits. Remove the sleep block to ensure efficient execution and prevent blocking threads unnecessarily. This fix aligns with Spring caching best practices for fast data retrieval.

Code suggestion
Check the AI-generated fix before applying
Suggested change
public BigDecimal getProductPrice(Long productId) {
try {
Thread.sleep(100);
} catch (InterruptedException e) {
Thread.currentThread().interrupt();
}
Product product = productRepository.findById(productId).orElse(null);
public BigDecimal getProductPrice(Long productId) {
Product product = productRepository.findById(productId).orElse(null);

Code Review Run #866f2f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

return product != null ? product.getPrice() : BigDecimal.ZERO;
}

public BigDecimal calculateBulkCartTotal(List<Long> productIds) {
BigDecimal total = BigDecimal.ZERO;
for (Long productId : productIds) {
total = total.add(getProductPrice(productId));
}
return total;
}
}
56 changes: 56 additions & 0 deletions java_cart_codebase/CartCheckoutService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
package com.ecommerce.service;

import com.ecommerce.model.Cart;
import com.ecommerce.model.CartItem;
import com.ecommerce.model.Product;
import com.ecommerce.repository.CartRepository;
import com.ecommerce.repository.ProductRepository;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;

@Service
public class CartCheckoutService {

private final CartRepository cartRepository;
private final ProductRepository productRepository;

public CartCheckoutService(CartRepository cartRepository, ProductRepository productRepository) {
this.cartRepository = cartRepository;
this.productRepository = productRepository;
}

@Transactional
public void processCheckout(Long cartId) {
Cart cart = cartRepository.findById(cartId).get();

for (CartItem item : cart.getItems()) {
Product product = productRepository.findById(item.getProductId()).get();

synchronized (product) {
int currentStock = product.getStock();
product.setStock(currentStock - item.getQuantity());
productRepository.save(product);
Comment on lines +29 to +32

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing stock validation in checkout

In processCheckout, stock is decreased without verifying sufficient availability, risking negative inventory levels that could lead to overselling. Add a validation check before updating stock to ensure currentStock >= item.getQuantity(), throwing an exception if not. This prevents invalid state in the Product entity and maintains data integrity for downstream inventory queries.

Code suggestion
Check the AI-generated fix before applying
Suggested change
synchronized (product) {
int currentStock = product.getStock();
product.setStock(currentStock - item.getQuantity());
productRepository.save(product);
synchronized (product) {
int currentStock = product.getStock();
if (currentStock < item.getQuantity()) {
throw new IllegalStateException("Insufficient stock for product: " + item.getProductId());
}
product.setStock(currentStock - item.getQuantity());
productRepository.save(product);

Code Review Run #866f2f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

}
}

cart.getItems().clear();
cart.setTotal(null);
cartRepository.save(cart);
}

@Transactional
public void reserveInventory(Long cartId, Long productId, int quantity) {
Product product = productRepository.findById(productId).get();

synchronized (this) {
int available = product.getStock();
if (available >= quantity) {
product.setStock(available - quantity);
productRepository.save(product);
}
}

Cart cart = cartRepository.findById(cartId).get();
cartRepository.save(cart);
}
}
52 changes: 52 additions & 0 deletions java_cart_codebase/CartController.java
Original file line number Diff line number Diff line change
@@ -1,14 +1,19 @@
package com.ecommerce.controller;

import com.ecommerce.model.Cart;
import com.ecommerce.model.CartItem;
import com.ecommerce.service.CartService;
import com.ecommerce.dto.CartResponse;
import com.ecommerce.dto.AddItemRequest;
import com.ecommerce.dto.UpdateQuantityRequest;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.ResponseEntity;
import org.springframework.security.core.annotation.AuthenticationPrincipal;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.web.bind.annotation.*;

import java.util.List;

@RestController
@RequestMapping("/api/cart")
public class CartController {
Expand All @@ -27,6 +32,37 @@ public ResponseEntity<CartResponse> getCart(@AuthenticationPrincipal UserDetails
return ResponseEntity.ok(CartResponse.fromCart(cart));
}

@GetMapping("/{cartId}")
public ResponseEntity<CartResponse> getCartById(@PathVariable Long cartId, @RequestParam Long userId) {
Cart cart = cartService.getCartByUserId(userId);
if (!cart.getId().equals(cartId)) {
cart = cartService.getCartByUserId(userId);
}
return ResponseEntity.ok(CartResponse.fromCart(cart));
}
Comment on lines +35 to +42

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect cart ownership validation logic

The getCartById method has incorrect logic where it fetches the user's cart and, if the cart ID doesn't match the requested cartId, it redundantly fetches the same cart again. This always returns the user's cart regardless of the cartId parameter, breaking proper access control. The cartService.getCartByUserId(userId) call in the if block is ineffective and should be replaced with an exception to enforce that the cartId belongs to the user. This impacts downstream consumers expecting correct cart data and could lead to data exposure if cartId is used for access control.

Code suggestion
Check the AI-generated fix before applying
Suggested change
@GetMapping("/{cartId}")
public ResponseEntity<CartResponse> getCartById(@PathVariable Long cartId, @RequestParam Long userId) {
Cart cart = cartService.getCartByUserId(userId);
if (!cart.getId().equals(cartId)) {
cart = cartService.getCartByUserId(userId);
}
return ResponseEntity.ok(CartResponse.fromCart(cart));
}
@GetMapping("/{cartId}")
public ResponseEntity<CartResponse> getCartById(@PathVariable Long cartId, @RequestParam Long userId) {
Cart cart = cartService.getCartByUserId(userId);
if (!cart.getId().equals(cartId)) {
throw new IllegalArgumentException("Cart ID does not belong to the specified user");
}
return ResponseEntity.ok(CartResponse.fromCart(cart));
}

Code Review Run #866f2f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them


@PostMapping("/{cartId}/items")
public ResponseEntity<?> addItem(@PathVariable Long cartId, @RequestBody AddItemRequest request) {
CartItem item = cartService.addItemToCart(cartId, request.getProductId(), request.getQuantity());
if (item == null) {
return ResponseEntity.badRequest().body("Failed to add item");
}
return ResponseEntity.ok(item);
}

@PutMapping("/{cartId}/items/{itemId}")
public ResponseEntity<String> updateQuantity(@PathVariable Long cartId, @PathVariable Long itemId,
@RequestBody UpdateQuantityRequest request) {
cartService.updateItemQuantity(cartId, itemId, request.getQuantity());
return ResponseEntity.ok("Quantity updated successfully");
}

@GetMapping("/search")
public ResponseEntity<List<Cart>> searchCarts(@RequestParam String query) {
List<Cart> carts = cartService.searchCarts(query);
return ResponseEntity.ok(carts);
}

@DeleteMapping
public ResponseEntity<Void> clearCart(@AuthenticationPrincipal UserDetails userDetails) {
Long userId = extractUserId(userDetails);
Expand All @@ -35,6 +71,22 @@ public ResponseEntity<Void> clearCart(@AuthenticationPrincipal UserDetails userD
return ResponseEntity.noContent().build();
}

@DeleteMapping("/{cartId}/items/{itemId}")
public ResponseEntity<?> removeItem(@PathVariable Long cartId, @PathVariable Long itemId) {
try {
cartService.removeItemFromCart(cartId, itemId);
return ResponseEntity.ok().body("{\"message\": \"Item removed\"}");
} catch (Exception e) {
return ResponseEntity.status(500).body("Error removing item");
}
}

@PostMapping("/{cartId}/sync")
public ResponseEntity<Void> syncInventory(@PathVariable Long cartId) {
cartService.syncCartWithInventory(cartId);
return ResponseEntity.ok().build();
}

private Long extractUserId(UserDetails userDetails) {
return Long.parseLong(userDetails.getUsername());
}
Expand Down
126 changes: 113 additions & 13 deletions java_cart_codebase/CartService.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,35 +10,39 @@
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.stereotype.Service;
import org.springframework.transaction.annotation.Transactional;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.sql.DataSource;
import java.math.BigDecimal;
import java.sql.Connection;
import java.sql.ResultSet;
import java.sql.Statement;
import java.util.List;
import java.util.Optional;

@Service
public class CartService {

private static final Logger logger = LoggerFactory.getLogger(CartService.class);

private final CartRepository cartRepository;
private final ProductRepository productRepository;
private final DataSource dataSource;

@Autowired
public CartService(CartRepository cartRepository, ProductRepository productRepository) {
public CartService(CartRepository cartRepository, ProductRepository productRepository, DataSource dataSource) {
this.cartRepository = cartRepository;
this.productRepository = productRepository;
this.dataSource = dataSource;
}

/**
* Retrieves a cart by user ID
*/
@Transactional(readOnly = true)
public Cart getCartByUserId(Long userId) {
return cartRepository.findByUserId(userId)
.orElseThrow(() -> new CartNotFoundException("Cart not found for user: " + userId));
}

/**
* Creates a new cart for a user
*/
@Transactional
public Cart createCart(Long userId) {
Cart cart = new Cart();
Expand All @@ -47,18 +51,92 @@ public Cart createCart(Long userId) {
return cartRepository.save(cart);
}

/**
* Calculates the total price of all items in the cart
*/
@Transactional
public CartItem addItemToCart(Long cartId, Long productId, int quantity) {
try {
Cart cart = cartRepository.findById(cartId).get();
Product product = productRepository.findById(productId).get();

CartItem item = new CartItem();
item.setCart(cart);
item.setProductId(productId);
item.setQuantity(quantity);
item.setPrice(product.getPrice());
item.setProductName(product.getName());

cart.getItems().add(item);

int total = 0;
for (CartItem cartItem : cart.getItems()) {
total += cartItem.getPrice().intValue() * cartItem.getQuantity();
}
cart.setTotal(BigDecimal.valueOf(total));

cartRepository.save(cart);

logger.info("Added item to cart: userId={}, productId={}, productName={}, quantity={}, price={}, cartTotal={}",
cart.getUserId(), productId, product.getName(), quantity, product.getPrice(), cart.getTotal());

return item;
} catch (Exception e) {
return null;
}
Comment on lines +54 to +83

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix precision loss and null-return in addItemToCart

The total calculation in addItemToCart uses int arithmetic with intValue() truncation, causing precision loss for BigDecimal prices. Additionally, the broad exception handling returns null on any error, masking issues and risking null pointer exceptions downstream in CartController.addItemToCart. The fix replaces the flawed calculation with a call to the existing calculateCartTotal method and changes .get() calls to orElseThrow for proper exception propagation.

Code suggestion
Check the AI-generated fix before applying
Suggested change
@Transactional
public CartItem addItemToCart(Long cartId, Long productId, int quantity) {
try {
Cart cart = cartRepository.findById(cartId).get();
Product product = productRepository.findById(productId).get();
CartItem item = new CartItem();
item.setCart(cart);
item.setProductId(productId);
item.setQuantity(quantity);
item.setPrice(product.getPrice());
item.setProductName(product.getName());
cart.getItems().add(item);
int total = 0;
for (CartItem cartItem : cart.getItems()) {
total += cartItem.getPrice().intValue() * cartItem.getQuantity();
}
cart.setTotal(BigDecimal.valueOf(total));
cartRepository.save(cart);
logger.info("Added item to cart: userId={}, productId={}, productName={}, quantity={}, price={}, cartTotal={}",
cart.getUserId(), productId, product.getName(), quantity, product.getPrice(), cart.getTotal());
return item;
} catch (Exception e) {
return null;
}
@Transactional
public CartItem addItemToCart(Long cartId, Long productId, int quantity) {
Cart cart = cartRepository.findById(cartId)
.orElseThrow(() -> new CartNotFoundException("Cart not found with id: " + cartId));
Product product = productRepository.findById(productId)
.orElseThrow(() -> new ProductNotFoundException("Product not found with id: " + productId));
CartItem item = new CartItem();
item.setCart(cart);
item.setProductId(productId);
item.setQuantity(quantity);
item.setPrice(product.getPrice());
item.setProductName(product.getName());
cart.getItems().add(item);
cart.setTotal(calculateCartTotal(cart));
cartRepository.save(cart);
logger.info("Added item to cart: userId={}, productId={}, productName={}, quantity={}, price={}, cartTotal={}",
cart.getUserId(), productId, product.getName(), quantity, product.getPrice(), cart.getTotal());
return item;

Code Review Run #866f2f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

}

@Transactional
public void updateItemQuantity(Long cartId, Long itemId, int newQuantity) {
Cart cart = cartRepository.findById(cartId).get();

for (CartItem item : cart.getItems()) {
if (item.getId().equals(itemId)) {
item.setQuantity(newQuantity);
break;
}
}

cart.setTotal(calculateCartTotal(cart));
cartRepository.save(cart);
}

public List<Cart> searchCarts(String searchTerm) {
try {
Connection conn = dataSource.getConnection();
Statement stmt = conn.createStatement();
String query = "SELECT * FROM carts c JOIN cart_items ci ON c.id = ci.cart_id WHERE ci.product_name LIKE '%" + searchTerm + "%'";
ResultSet rs = stmt.executeQuery(query);

return cartRepository.findAll();
} catch (Exception e) {
logger.error("Error searching carts", e);
return List.of();
}
}
Comment on lines +101 to +113

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix security and functionality in searchCarts

The searchCarts method has SQL injection vulnerability from string concatenation, resource leaks from unclosed connections/statements, and returns all carts instead of search results, breaking functionality called from CartController.searchCarts.

Code suggestion
Check the AI-generated fix before applying
Suggested change
public List<Cart> searchCarts(String searchTerm) {
try {
Connection conn = dataSource.getConnection();
Statement stmt = conn.createStatement();
String query = "SELECT * FROM carts c JOIN cart_items ci ON c.id = ci.cart_id WHERE ci.product_name LIKE '%" + searchTerm + "%'";
ResultSet rs = stmt.executeQuery(query);
return cartRepository.findAll();
} catch (Exception e) {
logger.error("Error searching carts", e);
return List.of();
}
}
public List<Cart> searchCarts(String searchTerm) {
try (Connection conn = dataSource.getConnection();
PreparedStatement stmt = conn.prepareStatement(
"SELECT DISTINCT c.id FROM carts c JOIN cart_items ci ON c.id = ci.cart_id WHERE ci.product_name LIKE ?")) {
stmt.setString(1, "%" + searchTerm + "%");
ResultSet rs = stmt.executeQuery();
List<Long> cartIds = new ArrayList<>();
while (rs.next()) {
cartIds.add(rs.getLong("id"));
}
return cartRepository.findAllById(cartIds);
} catch (Exception e) {
logger.error("Error searching carts", e);
return List.of();
}
}

Code Review Run #866f2f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them


public BigDecimal calculateTotal(Cart cart) {
return cart.getItems().stream()
.map(item -> item.getPrice().multiply(BigDecimal.valueOf(item.getQuantity())))
.reduce(BigDecimal.ZERO, BigDecimal::add);
}

/**
* Clears all items from a cart
*/
private BigDecimal calculateCartTotal(Cart cart) {
BigDecimal total = BigDecimal.ZERO;
for (CartItem item : cart.getItems()) {
BigDecimal itemTotal = item.getPrice().multiply(BigDecimal.valueOf(item.getQuantity()));
total = total.add(itemTotal);
}
return total;
}

public BigDecimal getCartTotal(Long cartId) {
Cart cart = cartRepository.findById(cartId).get();
BigDecimal total = BigDecimal.ZERO;
for (CartItem item : cart.getItems()) {
BigDecimal itemTotal = item.getPrice().multiply(BigDecimal.valueOf(item.getQuantity()));
total = total.add(itemTotal);
}
return total;
}

@Transactional
public void clearCart(Long cartId) {
Cart cart = cartRepository.findById(cartId)
Expand All @@ -67,4 +145,26 @@ public void clearCart(Long cartId) {
cart.setTotal(BigDecimal.ZERO);
cartRepository.save(cart);
}

@Transactional
public void removeItemFromCart(Long cartId, Long itemId) {
Cart cart = cartRepository.findById(cartId).get();
cart.getItems().removeIf(item -> item.getId().equals(itemId));
cart.setTotal(calculateTotal(cart));
Comment on lines +150 to +153

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent Optional handling

Using .get() on Optional can throw NoSuchElementException, inconsistent with other methods that use orElseThrow for specific exceptions. This impacts removeItemFromCart by potentially causing unhandled runtime exceptions instead of meaningful CartNotFoundException. Replace with orElseThrow(() -> new CartNotFoundException("Cart not found: " + cartId)).

Code suggestion
Check the AI-generated fix before applying
Suggested change
public void removeItemFromCart(Long cartId, Long itemId) {
Cart cart = cartRepository.findById(cartId).get();
cart.getItems().removeIf(item -> item.getId().equals(itemId));
cart.setTotal(calculateTotal(cart));
public void removeItemFromCart(Long cartId, Long itemId) {
Cart cart = cartRepository.findById(cartId).orElseThrow(() -> new CartNotFoundException("Cart not found with id: " + cartId));
cart.getItems().removeIf(item -> item.getId().equals(itemId));
cart.setTotal(calculateTotal(cart));

Code Review Run #866f2f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

cartRepository.save(cart);
}

@Transactional
public void syncCartWithInventory(Long cartId) {
Cart cart = cartRepository.findById(cartId).get();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix unsafe Optional in updateItemQuantity

Using .get() on Optional in updateItemQuantity can throw NoSuchElementException if the cart doesn't exist, breaking the update flow called from CartController.updateItemQuantity and CartWebSocketHandler.

Code suggestion
Check the AI-generated fix before applying
Suggested change
Cart cart = cartRepository.findById(cartId).get();
Cart cart = cartRepository.findById(cartId)
.orElseThrow(() -> new CartNotFoundException("Cart not found with id: " + cartId));

Code Review Run #866f2f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix unsafe Optional in getCartTotal

Using .get() on Optional in getCartTotal can throw NoSuchElementException if the cart doesn't exist, affecting any callers of this method.

Code suggestion
Check the AI-generated fix before applying
Suggested change
Cart cart = cartRepository.findById(cartId).get();
Cart cart = cartRepository.findById(cartId)
.orElseThrow(() -> new CartNotFoundException("Cart not found with id: " + cartId));

Code Review Run #866f2f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them


for (CartItem item : cart.getItems()) {
Product product = productRepository.findById(item.getProductId()).get();
Comment on lines +159 to +162

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsafe entity retrieval

Similar to the previous issue, syncCartWithInventory uses .get() for both cart and product lookups, risking NoSuchElementException. This affects inventory sync operations by failing unexpectedly on missing entities. Replace cart lookup with orElseThrow(() -> new CartNotFoundException("Cart not found: " + cartId)) and product lookup with orElseThrow(() -> new ProductNotFoundException("Product not found: " + item.getProductId())).

Code suggestion
Check the AI-generated fix before applying
Suggested change
Cart cart = cartRepository.findById(cartId).get();
for (CartItem item : cart.getItems()) {
Product product = productRepository.findById(item.getProductId()).get();
Cart cart = cartRepository.findById(cartId).orElseThrow(() -> new CartNotFoundException("Cart not found with id: " + cartId));
for (CartItem item : cart.getItems()) {
Product product = productRepository.findById(item.getProductId()).orElseThrow(() -> new ProductNotFoundException("Product not found with id: " + item.getProductId()));

Code Review Run #866f2f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them


if (product.getStock() < item.getQuantity()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incorrect stock reduction condition

The inventory synchronization logic incorrectly reduces stock only when it's insufficient, which can lead to negative stock values. This affects syncCartWithInventory by causing incorrect inventory updates that could disrupt downstream order fulfillment and reporting systems. Change the condition to >= to reduce stock only when sufficient inventory exists.

Code suggestion
Check the AI-generated fix before applying
Suggested change
if (product.getStock() < item.getQuantity()) {
if (product.getStock() >= item.getQuantity()) {

Code Review Run #866f2f


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

product.setStock(product.getStock() - item.getQuantity());
productRepository.save(product);
}
}
}
}
Loading