Need little clarification about GetAllDiscountsAsync method nop4.5.x

This topic was automatically closed 365 days after the last reply. New replies are no longer allowed.
Il y a 1 an
//we load all discounts, and filter them using "discountType" parameter later (in memory)
            //we do it because we know that this method is invoked several times per HTTP request with distinct "discountType" parameter
            //that's why let's access the database only once
            var discounts = (await _discountRepository.GetAllAsync(query =>
            {
                if (!showHidden)
                    query = query.Where(discount =>
                        (!discount.StartDateUtc.HasValue || discount.StartDateUtc <= DateTime.UtcNow) &&
                        (!discount.EndDateUtc.HasValue || discount.EndDateUtc >= DateTime.UtcNow));

                //filter by coupon code
                if (!string.IsNullOrEmpty(couponCode))
                    query = query.Where(discount => discount.CouponCode == couponCode);

                //filter by name
                if (!string.IsNullOrEmpty(discountName))
                    query = query.Where(discount => discount.Name.Contains(discountName));

                query = query.OrderBy(discount => discount.Name).ThenBy(discount => discount.Id);

                return query;
            }, cache => cache.PrepareKeyForDefaultCache(NopDiscountDefaults.DiscountAllCacheKey,
                showHidden, couponCode ?? string.Empty, discountName ?? string.Empty)))
            .AsQueryable();


This piece of code load all types of discounts depending on the StartDateUtc,EndDateUtc,couponCode, and discountName from the database then keep that data in the cache(memory/distributed) then by doing AsQueryable it comes to memory and applies others filter like discountType, discountStatusId, etc later of the method.

Is it true or I misunderstood?
Il y a 1 an
According to their comments, it seems that that is what is intended, but similar to your other post, I question whether it works to " keep that data in the cache".  GetAllAsync() returns an IList.  Yet, what is stored in the cache is an IQueryable, which is an 'expression tree'.   So, I would think that the DB would be accessed every time the method is called.  I could be wrong;  I'd have to 'debug' (profile the SQL server) to be sure.
Il y a 1 an
Hi, Md. Abu Sina. I'll bring clarity. Indeed, the result of executing the _discountRepository.GetAllAsync method is placed in the cache; it is an instance of IList<Discount>. That is, the required query is sent to the database only once. If necessary, the data is retrieved from the cache and, for the convenience of work, it is stored in IQueryable, but this instance is not connected to the database in any way.
Il y a 1 an
Sergei-k wrote:
Hi, Md. Abu Sina. I'll bring clarity. Indeed, the result of executing the _discountRepository.GetAllAsync method is placed in the cache; it is an instance of IList<Discount>. That is, the required query is sent to the database only once. If necessary, the data is retrieved from the cache and, for the convenience of work, it is stored in IQueryable, but this instance is not connected to the database in any way.


Agreed. So if I have a 3-5k active discount for the first time it will go to the database and store this data large JSON as byte array to the distributed cache and from the next call, it will take this same large byte from the distributed cache if the distributed cache has been enabled?
Il y a 1 an
right, if we imagine the situation that you have 3k active discounts and they will not be filtered by date, name and coupon in any way, then everything will certainly be that way. And most likely your distributed cache will simply not cope with such a load.
Il y a 1 an
Sergei-k wrote:
right, if we imagine the situation that you have 3k active discounts and they will not be filtered by date, name and coupon in any way, then everything will certainly be that way. And most likely your distributed cache will simply not cope with such a load.


Thank you so much for answering the query. I am agreed and aligned with your answer. I have last one more question. Can not we add more filters to the expression and narrowed down the result? Something like below.


var query = _discountRepository.Table;
            if (!showHidden)
                query = query.Where(discount =>
                    (!discount.StartDateUtc.HasValue || discount.StartDateUtc <= DateTime.UtcNow) &&
                    (!discount.EndDateUtc.HasValue || discount.EndDateUtc >= DateTime.UtcNow));

            //filter by coupon code
            if (!string.IsNullOrEmpty(couponCode))
                query = query.Where(discount => discount.CouponCode == couponCode);

            //filter by name
            if (!string.IsNullOrEmpty(discountName))
                query = query.Where(discount => discount.Name.Contains(discountName));


            if (discountStatusId.HasValue)
            {
                query = query.Where(x => x.Active == discountStatusId.Value);
            }

            //we know that this method is usually invoked multiple times
            //that's why we filter discounts by type and dates on the application layer

            if (discountType.HasValue)
            {
                query = query.Where(discount => discount.DiscountTypeId == (int)discountType.Value);
            }

            //filter coupin code
            if (requiredCouponCodeOnly)
            {
                query = query.Where(x => x.RequiresCouponCode);
            }

            //filter by dates
            if (startDateUtc.HasValue)
                query = query.Where(discount =>
                    !discount.StartDateUtc.HasValue || discount.StartDateUtc >= startDateUtc.Value);
            if (endDateUtc.HasValue)
                query = query.Where(discount =>
                    !discount.EndDateUtc.HasValue || discount.EndDateUtc <= endDateUtc.Value);

            //hide archived
            if (hideArchived)
            {
                query = query.Where(x => !x.Archived);
            }


            if (discountType == null)
            {
                query = query.OrderBy(discount => discount.Name).ThenBy(discount => discount.Id);
                query = query.OrderByDescending(d => d.Id);

                var cacheKey = _staticCacheManager.PrepareKeyForDefaultCache(NopDiscountDefaults.DiscountAllCacheKey,
                     showHidden, couponCode ?? string.Empty, discountName ?? string.Empty);

                var activeDiscount = await _cacheManager.GetAsync(cacheKey, async () =>
                {
                    return await query.ToListAsync();
                });

                return activeDiscount;
            }
            else
            {
                query = query.OrderBy(discount => discount.Name).ThenBy(discount => discount.Id);
                query = query.OrderByDescending(d => d.Id);


                var cacheKey = _staticCacheManager.PrepareKeyForDefaultCache(NopDiscountDefaults.DiscountAllCacheKeyByType,
                    showHidden, couponCode ?? string.Empty, discountName ?? string.Empty, discountType.Value);

                var activeDiscount = await _cacheManager.GetAsync(cacheKey, async () =>
                {
                    return await query.ToListAsync();
                });

                return activeDiscount;
            }
Il y a 1 an
(Mea culpa on my prior response.  I never drilled into the code to see that the EntityRepository GetAllAsync() does the query.ToListAsync() )

If you "add more filters to the expression and narrowed down the result" but the filters are not part of the cache key, then I don't think it will work.
Il y a 1 an
New York wrote:
(Mea culpa on my prior response.  I never drilled into the code to see that the EntityRepository GetAllAsync() does the query.ToListAsync() )

If you "add more filters to the expression and narrowed down the result" but the filters are not part of the cache key, then I don't think it will work.


Key is different depending on cache type if you closely look at the else method.



                var cacheKey = _staticCacheManager.PrepareKeyForDefaultCache(NopDiscountDefaults.DiscountAllCacheKeyByType,
                    showHidden, couponCode ?? string.Empty, discountName ?? string.Empty, discountType.Value);
Il y a 1 an
Hi. Of course, you can add parameters to the caching key and thereby reduce the amount of data sent simultaneously from the distributed cache. But you will increase the number of database accesses, and then the question arises who is better able to handle the load of your database, or a distributed cache system

sina.islam wrote:
right, if we imagine the situation that you have 3k active discounts and they will not be filtered by date, name and coupon in any way, then everything will certainly be that way. And most likely your distributed cache will simply not cope with such a load.

Thank you so much for answering the query. I am agreed and aligned with your answer. I have last one more question. Can not we add more filters to the expression and narrowed down the result? Something like below....

Il y a 1 an
RE: "Key is different depending on cache type..."
Yes, I saw that, but it's not accounting for the other criteria.  I.e. your key has
...PrepareKeyForDefaultCache(
   NopDiscountDefaults.DiscountAllCacheKeyByType,
   showHidden,
   couponCode,
   discountName,
   discountType.Value);

But in your 'query' you may also be using
  discountStatusId
  requiredCouponCodeOnly
  startDateUtc
  endDateUtc.

I.e. look at the original code.  All the potential search criteria (showHidden,   couponCode, discountName) are part of the key.
This topic was automatically closed 365 days after the last reply. New replies are no longer allowed.