Skip to content
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

Issue #1168 storefront only get product added to warehouse #1246

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
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
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,5 @@
import org.springframework.boot.context.properties.ConfigurationProperties;

@ConfigurationProperties(prefix = "yas.services")
public record ServiceUrlConfig(String media, String product) {
public record ServiceUrlConfig(String media, String product, String inventory) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ public final class ApiConstant {

public static final String STOCK_HISTORY_URL = "/backoffice/stocks/histories";

public static final String STOCK_URL = "/backoffice/stocks";
public static final String BACKOFFICE_STOCK_URL = "/backoffice/stocks";
public static final String STOREFRONT_STOCK_URL = "/storefront/stocks";
public static final String CODE_200 = "200";
public static final String OK = "Ok";
public static final String CODE_404 = "404";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,17 @@
import org.springframework.web.bind.annotation.RestController;

@RestController
@RequestMapping(ApiConstant.STOCK_URL)
@RequiredArgsConstructor
public class StockController {
private final StockService stockService;

@PostMapping
@PostMapping(ApiConstant.BACKOFFICE_STOCK_URL)
public ResponseEntity<Void> addProductIntoWarehouse(@RequestBody List<StockPostVm> postVms) {
stockService.addProductIntoWarehouse(postVms);
return ResponseEntity.noContent().build();
}

@GetMapping
@GetMapping(ApiConstant.BACKOFFICE_STOCK_URL)
public ResponseEntity<List<StockVm>> getStocksByWarehouseIdAndProductNameAndSku(
@RequestParam(name = "warehouseId") Long warehouseId,
@RequestParam(required = false) String productName,
Expand All @@ -40,10 +39,16 @@ public ResponseEntity<List<StockVm>> getStocksByWarehouseIdAndProductNameAndSku(
));
}

@PutMapping
@PutMapping(ApiConstant.BACKOFFICE_STOCK_URL)
public ResponseEntity<Void> updateProductQuantityInStock(@RequestBody StockQuantityUpdateVm requestBody) {
stockService.updateProductQuantityInStock(requestBody);

return ResponseEntity.ok().build();
}

@GetMapping(ApiConstant.STOREFRONT_STOCK_URL + "/products-in-warehouse")
public List<Long> getProductsInWarehouse() {
return stockService.findProductIdsAddedWarehouse();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,7 @@ List<Stock> findByWarehouseIdAndProductIdIn(Long warehouseId,
List<Long> productIds);

boolean existsByWarehouseIdAndProductId(Long warehouseId, Long productId);

@Query("SELECT distinct s.productId FROM Stock s")
List<Long> findProductIdsAddedWarehouse();
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have cases where quantity = 0? Those records should be excluded, right?
findProductIdsInStock may be a better name.

}
Original file line number Diff line number Diff line change
Expand Up @@ -144,4 +144,8 @@ public void updateProductQuantityInStock(final StockQuantityUpdateVm requestBody
productService.updateProductQuantity(productQuantityPostVms);
}
}

public List<Long> findProductIdsAddedWarehouse() {
return stockRepository.findProductIdsAddedWarehouse();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -214,17 +214,6 @@ void test_getProductListBackoffice_shouldReturnProductList() {
.log().ifValidationFails();
}

@Test
void test_getProductFeature_shouldReturnProductList() {
getGivenSpecificationWithAdmin()
.when()
.get(PRODUCT_STOREFRONT_URL + "/featured")
.then()
.statusCode(HttpStatus.OK.value())
.body("productList", hasSize(2))
.log().ifValidationFails();
}

@Test
void test_getProductFeatureById_shouldReturnProductList() {
Long productOneId = productOne.getId();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@ class ProductServiceIT {
private MediaService mediaService;
@Autowired
private ProductService productService;
@MockBean
private InventoryService inventoryService;
private List<Product> products;
private List<Category> categoryList;
private List<ProductCategory> productCategoryList;
Expand All @@ -85,9 +87,10 @@ class ProductServiceIT {

@BeforeEach
void setUp() {
generateTestData();
noFileMediaVm = new NoFileMediaVm(1L, "caption", "fileName", "mediaType", "url");
when(mediaService.getMedia(1L)).thenReturn(noFileMediaVm);
generateTestData();
when(inventoryService.getProductIdsAddedWarehouse()).thenReturn(products.stream().map(p->p.getId()).toList());
}

private void generateTestData() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,26 @@ Page<Product> getProductsWithFilter(@Param("productName") String productName,
List<Product> findAllByIdIn(List<Long> productIds);

@Query(value = "FROM Product p WHERE p.isFeatured = TRUE "
+ "AND p.isVisibleIndividually = TRUE "
+ "AND p.isPublished = TRUE ORDER BY p.id ASC ")
Page<Product> getFeaturedProduct(Pageable pageable);
+ "AND p.isVisibleIndividually = TRUE "
+ "AND p.isPublished = TRUE "
+ "AND p.id IN (:productIds) "
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move p.id IN (:productIds) to the top condition to utilize the index on the id column.
Passing a large list of productIds in the IN clause may result in a query limit error.
Consider applying batching here.

+ "ORDER BY p.id ASC ")
Page<Product> getFeaturedProductByProductIds(@Param("productIds") List<Long> productIds, Pageable pageable);

@Query(value = "SELECT p FROM Product p LEFT JOIN p.productCategories pc LEFT JOIN pc.category c "
@Query(value = "SELECT distinct p FROM Product p LEFT JOIN p.productCategories pc LEFT JOIN pc.category c "
+ "WHERE LOWER(p.name) LIKE %:productName% "
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move the indexed condition id to the top.
Do we have an index on the product name? If so, using LOWER(p.name) may prevent the index use.

+ "AND p.id IN (:productIds) "
+ "AND (c.slug = :categorySlug OR (:categorySlug IS NULL OR :categorySlug = '')) "
+ "AND (:startPrice IS NULL OR p.price >= :startPrice) "
+ "AND (:endPrice IS NULL OR p.price <= :endPrice) "
+ "AND p.isVisibleIndividually = TRUE "
+ "AND p.isPublished = TRUE "
+ "AND p.isVisibleIndividually = true "
+ "AND p.isPublished = true "
+ "ORDER BY p.id ASC ")
Page<Product> findByProductNameAndCategorySlugAndPriceBetween(@Param("productName") String productName,
@Param("categorySlug") String categorySlug,
@Param("startPrice") Double startPrice,
@Param("endPrice") Double endPrice,
@Param("productIds") List<Long> productIds,
Pageable pageable);

@Query(value = "SELECT p FROM Product p "
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
package com.yas.product.service;

import com.yas.commonlibrary.config.ServiceUrlConfig;
import io.github.resilience4j.circuitbreaker.annotation.CircuitBreaker;
import io.github.resilience4j.retry.annotation.Retry;
import java.net.URI;
import java.util.List;
import lombok.RequiredArgsConstructor;
import org.springframework.core.ParameterizedTypeReference;
import org.springframework.stereotype.Service;
import org.springframework.web.client.RestClient;
import org.springframework.web.util.UriComponentsBuilder;

@Service
@RequiredArgsConstructor
public class InventoryService extends AbstractCircuitBreakFallbackHandler {
private final RestClient restClient;
private final ServiceUrlConfig serviceUrlConfig;

@Retry(name = "restApi")
@CircuitBreaker(name = "restCircuitBreaker", fallbackMethod = "handleInventoryFallback")
public List<Long> getProductIdsAddedWarehouse() {
final URI url = UriComponentsBuilder.fromHttpUrl(serviceUrlConfig.inventory())
.path("/storefront/stocks/products-in-warehouse").buildAndExpand().toUri();
return restClient.get()
.uri(url)
.retrieve()
.body(new ParameterizedTypeReference<>() {
});
}

private List<Long> handleInventoryFallback(Throwable throwable) throws Throwable {
return handleTypedFallback(throwable);
}
}
40 changes: 15 additions & 25 deletions product/src/main/java/com/yas/product/service/ProductService.java
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@
import java.util.function.BiFunction;
import java.util.function.Function;
import java.util.stream.Collectors;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.commons.collections4.CollectionUtils;
import org.apache.commons.collections4.ListUtils;
Expand All @@ -80,6 +81,7 @@
@Service
@Transactional
@Slf4j
@RequiredArgsConstructor
public class ProductService {
private static final String NONE_GROUP = "None group";
private final ProductRepository productRepository;
Expand All @@ -92,28 +94,7 @@
private final ProductOptionValueRepository productOptionValueRepository;
private final ProductOptionCombinationRepository productOptionCombinationRepository;
private final ProductRelatedRepository productRelatedRepository;

public ProductService(ProductRepository productRepository,
MediaService mediaService,
BrandRepository brandRepository,
ProductCategoryRepository productCategoryRepository,
CategoryRepository categoryRepository,
ProductImageRepository productImageRepository,
ProductOptionRepository productOptionRepository,
ProductOptionValueRepository productOptionValueRepository,
ProductOptionCombinationRepository productOptionCombinationRepository,
ProductRelatedRepository productRelatedRepository) {
this.productRepository = productRepository;
this.mediaService = mediaService;
this.brandRepository = brandRepository;
this.categoryRepository = categoryRepository;
this.productCategoryRepository = productCategoryRepository;
this.productImageRepository = productImageRepository;
this.productOptionRepository = productOptionRepository;
this.productOptionValueRepository = productOptionValueRepository;
this.productOptionCombinationRepository = productOptionCombinationRepository;
this.productRelatedRepository = productRelatedRepository;
}
private final InventoryService inventoryService;

public ProductGetDetailVm createProduct(ProductPostVm productPostVm) {
validateProductVm(productPostVm);
Expand Down Expand Up @@ -769,9 +750,15 @@
}

public ProductFeatureGetVm getListFeaturedProducts(int pageNo, int pageSize) {
Pageable pageable = PageRequest.of(pageNo, pageSize);

List<ProductThumbnailGetVm> productThumbnailVms = new ArrayList<>();
Page<Product> productPage = productRepository.getFeaturedProduct(pageable);
List<Long> productIds = inventoryService.getProductIdsAddedWarehouse();
Copy link
Contributor

Choose a reason for hiding this comment

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

The current approach of fetching all available products in real-time should work. However, I don't like this approach so much as it impacts the responsibility if the number of products or requests is high. Please consider caching or an event-driven approach.

if (productIds.isEmpty()) {
return new ProductFeatureGetVm(productThumbnailVms, 0);
}

Pageable pageable = PageRequest.of(pageNo, pageSize);
Page<Product> productPage = productRepository.getFeaturedProductByProductIds(productIds, pageable);
List<Product> products = productPage.getContent();
for (Product product : products) {
productThumbnailVms.add(new ProductThumbnailGetVm(
Expand Down Expand Up @@ -875,11 +862,14 @@
Double endPrice
) {
Pageable pageable = PageRequest.of(pageNo, pageSize);
List<Long> productIds = inventoryService.getProductIdsAddedWarehouse();
if (productIds.isEmpty())
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add the block {} for this if.

return new ProductsGetVm(new ArrayList<>(), 0, 0, 0, 0, true);

Page<Product> productPage;
productPage = productRepository.findByProductNameAndCategorySlugAndPriceBetween(
productName.trim().toLowerCase(),
categorySlug.trim(), startPrice, endPrice, pageable);
categorySlug.trim(), startPrice, endPrice, productIds, pageable);

Check warning on line 872 in product/src/main/java/com/yas/product/service/ProductService.java

View workflow job for this annotation

GitHub Actions / Checkstyle

com.puppycrawl.tools.checkstyle.checks.blocks.NeedBracesCheck

'if' construct must use '{}'s.

List<ProductThumbnailGetVm> productThumbnailVms = new ArrayList<>();
List<Product> products = productPage.getContent();
Expand Down
1 change: 1 addition & 0 deletions product/src/main/resources/application.properties
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ spring.security.oauth2.resourceserver.jwt.issuer-uri=http://identity/realms/Yas

yas.services.media=http://api.yas.local/media
yas.services.rating=http://api.yas.local/rating
yas.services.inventory=http://api.yas.local/inventory

spring.datasource.driver-class-name=org.postgresql.Driver
spring.datasource.url=jdbc:postgresql://localhost:5432/product
Expand Down
4 changes: 2 additions & 2 deletions storefront/modules/home/components/FeaturedProduct.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ const FeaturedProduct = () => {
<Container className="featured-product-container">
<div className="title">Featured products</div>
<Row xs={5} xxl={6}>
{products.length > 0 &&
products.map((product) => (
{products?.length > 0 &&
products?.map((product) => (
<Col key={product.id}>
<ProductCard product={product} />
</Col>
Expand Down
2 changes: 1 addition & 1 deletion storefront/pages/products/[slug].tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export const getServerSideProps: GetServerSideProps = async (

// fetch product by slug
const product = await getProductDetail(slug as string);
if (!product.id) return { notFound: true };
if (!product?.id) return { notFound: true };

const productOptions: ProductOptions[] = [];
let productVariations: ProductVariation[] = [];
Expand Down
4 changes: 2 additions & 2 deletions storefront/pages/products/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -210,8 +210,8 @@ const ProductList = () => {
<div className="products-list" style={{ marginTop: 10 }}>
{/* PRODUCT VIEW */}
<Row xs={5} style={{ padding: '0 10px' }}>
{products.length > 0 &&
products.map((product) => (
{products?.length > 0 &&
products?.map((product) => (
<ProductCard
className={['products-page']}
product={product}
Expand Down
Loading