fix: cache regression

Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>

Author: ulemons

services/libs/data-access-layer/src/members/base.ts

@@ -195,23 +195,23 @@ export async function queryMembersAdvanced(
   // Initialize cache
   const cache = new MemberQueryCache(redis)
 
-  // Build cache key
+  // Build cache key — countOnly is excluded so count and full-result caches share the same key.
+  // Normalize empty search to null so "" and null produce the same key (buildSearchCTE treats them identically).
   const cacheKey = cache.buildCacheKey({
-    countOnly,
     fields,
     filter,
     include,
     includeAllAttributes,
     limit,
     offset,
     orderBy,
-    search,
+    search: search || null,
     segmentId,
   })
 
   // Try to get from cache first
   const cachedResult = countOnly ? null : await cache.get(cacheKey)
-  const cachedCount = countOnly ? null : await cache.getCount(cacheKey)
+  const cachedCount = countOnly ? await cache.getCount(cacheKey) : null
 
   if (cachedResult) {
     refreshCacheInBackground(bgQx, redis, cacheKey, {
@@ -343,7 +343,9 @@ export async function executeQuery(
     withAggregates,
     searchConfig,
     filterString,
-    includeMemberOrgs: include.memberOrganizations,
+    // Count never needs org data in SELECT — filterHasMo inside buildCountQuery already
+    // handles the case where the filter itself references mo.* columns.
+    includeMemberOrgs: false,
   })
 
   if (countOnly) {
@@ -537,7 +539,10 @@ export async function executeQuery(
 
   const result = { rows, count, limit, offset }
 
-  await cache.set(cacheKey, result, 21600) // 6 hours TTL
+  await Promise.all([
+    cache.set(cacheKey, result, 21600), // 6 hours TTL
+    cache.setCount(cacheKey, count, 21600), // also warm count cache so countOnly=true hits next time
+  ])
 
   return result
 }

services/libs/data-access-layer/src/members/queryBuilder.ts

@@ -464,9 +464,13 @@ export const buildCountQuery = ({
     searchConfig.join,
   ].filter(Boolean)
 
+  // When withAggregates is true the join is on memberSegmentsAgg_pkey (unique on memberId+segmentId),
+  // so there are no duplicate m.id values — COUNT(*) is equivalent to COUNT(DISTINCT m.id) but cheaper.
+  const countExpr = withAggregates ? 'COUNT(*)' : 'COUNT(DISTINCT m.id)'
+
   return `
     ${ctes.length > 0 ? `WITH ${ctes.join(',\n')}` : ''}
-    SELECT COUNT(DISTINCT m.id) AS count
+    SELECT ${countExpr} AS count
     FROM members m
     ${joins.join('\n')}
     WHERE (${filterString})

services/libs/data-access-layer/src/members/queryCache.ts

@@ -26,7 +26,6 @@ export class MemberQueryCache {
   }
 
   buildCacheKey(params: {
-    countOnly?: boolean
     fields?: string[]
     filter?: Record<string, unknown>
     include?: IncludeOptions
@@ -39,7 +38,6 @@ export class MemberQueryCache {
   }): string {
     const cleanParams = Object.fromEntries(
       Object.entries({
-        countOnly: params.countOnly,
         fields: params.fields?.sort(),
         filter: params.filter,
         include: params.include,
@@ -90,7 +88,11 @@ export class MemberQueryCache {
   }
 
   async setCount(cacheKey: string, count: number, ttlSeconds: number): Promise<void> {
-    await this.countCache.set(cacheKey, count.toString(), ttlSeconds)
+    try {
+      await this.countCache.set(cacheKey, count.toString(), ttlSeconds)
+    } catch (error) {
+      log.warn('Error saving count to cache', { error })
+    }
   }
 
   async invalidateAll(): Promise<void> {