fix: resolve query cache collision and bulk refresh issues

- Fix query cache key generation with deep object sorting
  Previously only top-level keys were sorted, causing cache collisions
  between similar queries (e.g. $phrase vs $phrase_prefix)
- Remove manual refresh handling in patch-bulk
  Elasticsearch bulk API natively supports refresh parameter
- Improve GitHub Actions Elasticsearch health check
  Wait for yellow/green cluster status instead of just connectivity

Author: marshallswain

.github/workflows/test-matrix.yml

@@ -2,9 +2,9 @@ name: Test Matrix
 
 on:
   push:
-    branches: [ main, master, dove ]
+    branches: [main, master, dove]
   pull_request:
-    branches: [ main, master, dove ]
+    branches: [main, master, dove]
 
 jobs:
   test:
@@ -18,48 +18,68 @@ jobs:
     name: Node ${{ matrix.node-version }} - ES ${{ matrix.elasticsearch-version }}
 
     steps:
-    - uses: actions/checkout@v3
+      - uses: actions/checkout@v3
 
-    - name: Use Node.js ${{ matrix.node-version }}
-      uses: actions/setup-node@v3
-      with:
-        node-version: ${{ matrix.node-version }}
+      - name: Use Node.js ${{ matrix.node-version }}
+        uses: actions/setup-node@v3
+        with:
+          node-version: ${{ matrix.node-version }}
 
-    - name: Start Elasticsearch ${{ matrix.elasticsearch-version }}
-      run: |
-        docker run -d \
-          --name elasticsearch \
-          -p 9200:9200 \
-          -e "discovery.type=single-node" \
-          -e "xpack.security.enabled=false" \
-          -e "xpack.security.enrollment.enabled=false" \
-          docker.elastic.co/elasticsearch/elasticsearch:${{ matrix.elasticsearch-version }}
+      - name: Start Elasticsearch ${{ matrix.elasticsearch-version }}
+        run: |
+          docker run -d \
+            --name elasticsearch \
+            -p 9200:9200 \
+            -e "discovery.type=single-node" \
+            -e "xpack.security.enabled=false" \
+            -e "xpack.security.enrollment.enabled=false" \
+            docker.elastic.co/elasticsearch/elasticsearch:${{ matrix.elasticsearch-version }}
 
-    - name: Wait for Elasticsearch
-      run: |
-        for i in {1..30}; do
-          if curl -s "http://localhost:9200/_cluster/health" > /dev/null 2>&1; then
-            echo "Elasticsearch is ready"
-            break
-          fi
-          echo "Waiting for Elasticsearch..."
-          sleep 5
-        done
+      - name: Wait for Elasticsearch
+        run: |
+          echo "Waiting for Elasticsearch to be ready..."
+          for i in {1..60}; do
+            # Check cluster health status
+            HEALTH=$(curl -s "http://localhost:9200/_cluster/health" 2>/dev/null || echo "")
+            if [ ! -z "$HEALTH" ]; then
+              STATUS=$(echo $HEALTH | grep -o '"status":"[^"]*"' | cut -d'"' -f4)
+              echo "Attempt $i: Cluster status is '$STATUS'"
 
-    - name: Install dependencies
-      run: npm ci
+              # Wait for yellow or green status (yellow is ok for single-node)
+              if [ "$STATUS" = "yellow" ] || [ "$STATUS" = "green" ]; then
+                echo "Elasticsearch is ready!"
+                # Give it a bit more time to fully stabilize
+                sleep 5
+                curl -s "http://localhost:9200/_cluster/health?pretty"
+                break
+              fi
+            else
+              echo "Attempt $i: Elasticsearch not responding yet..."
+            fi
 
-    - name: Build
-      run: npm run build
+            if [ $i -eq 60 ]; then
+              echo "ERROR: Elasticsearch failed to become ready after 5 minutes"
+              docker logs elasticsearch
+              exit 1
+            fi
 
-    - name: Run tests
-      run: |
-        ES_VERSION=${{ matrix.elasticsearch-version }} \
-        ELASTICSEARCH_URL=http://localhost:9200 \
-        npm run mocha
+            sleep 5
+          done
 
-    - name: Upload coverage
-      if: matrix.node-version == '20' && matrix.elasticsearch-version == '8.15.0'
-      uses: codecov/codecov-action@v3
-      with:
-        file: ./coverage/lcov.info
+      - name: Install dependencies
+        run: npm ci
+
+      - name: Build
+        run: npm run build
+
+      - name: Run tests
+        run: |
+          ES_VERSION=${{ matrix.elasticsearch-version }} \
+          ELASTICSEARCH_URL=http://localhost:9200 \
+          npm run mocha
+
+      - name: Upload coverage
+        if: matrix.node-version == '20' && matrix.elasticsearch-version == '8.15.0'
+        uses: codecov/codecov-action@v3
+        with:
+          file: ./coverage/lcov.info

package-lock.json

@@ -55,36 +55,16 @@ function prepareBulkUpdateParams(
   operations: Array<Record<string, unknown>>,
   index: string,
   requestParams: ElasticsearchServiceParams
-): { params: Record<string, unknown>; needsRefresh: boolean } {
+): Record<string, unknown> {
   // PERFORMANCE: Merge esParams with per-operation refresh override
-  const params = Object.assign(
+  // Note: Elasticsearch bulk API supports refresh parameter directly
+  return Object.assign(
     {
       index,
       body: operations
     },
     mergeESParamsWithRefresh(service.esParams, requestParams)
   )
-
-  // Remove refresh from bulk params but return it separately
-  const needsRefresh = params.refresh as boolean
-  delete params.refresh
-
-  return { params, needsRefresh }
-}
-
-/**
- * Handles refresh if needed after bulk operation
- */
-async function handleRefresh(
-  service: ElasticAdapterInterface,
-  bulkResult: unknown,
-  needsRefresh: boolean,
-  index: string
-): Promise<unknown> {
-  if (needsRefresh) {
-    await service.Model.indices.refresh({ index })
-  }
-  return bulkResult
 }
 
 /**
@@ -201,19 +181,14 @@ export async function patchBulk(
   const operations = createBulkOperations(service, found, data, index)
 
   // Step 3: Prepare and execute bulk update
-  const { params: bulkUpdateParams, needsRefresh } = prepareBulkUpdateParams(
-    service,
-    operations,
-    index,
-    params
-  )
-
-  let bulkResult = (await service.Model.bulk(bulkUpdateParams as never)) as unknown as Record<string, unknown>
+  const bulkUpdateParams = prepareBulkUpdateParams(service, operations, index, params)
 
-  // Step 4: Handle refresh if needed
-  bulkResult = (await handleRefresh(service, bulkResult, needsRefresh, index)) as Record<string, unknown>
+  const bulkResult = (await service.Model.bulk(bulkUpdateParams as never)) as unknown as Record<
+    string,
+    unknown
+  >
 
-  // Step 5: Get updated document IDs
+  // Step 4: Get updated document IDs
   const updatedIds = getUpdatedIds(bulkResult)
 
   if (updatedIds.length === 0) {

src/methods/patch-bulk.ts

@@ -13,15 +13,39 @@ const queryCache = new Map<string, { result: ESQuery | null; timestamp: number }
 const CACHE_MAX_SIZE = 1000
 const CACHE_MAX_AGE = 5 * 60 * 1000 // 5 minutes
 
+/**
+ * Recursively sort object keys for deterministic JSON serialization
+ * @param obj - Object to normalize
+ * @returns Normalized object with sorted keys
+ */
+function normalizeObject(obj: unknown): unknown {
+  if (obj === null || typeof obj !== 'object') {
+    return obj
+  }
+
+  if (Array.isArray(obj)) {
+    return obj.map(normalizeObject)
+  }
+
+  const sorted: Record<string, unknown> = {}
+  Object.keys(obj as object)
+    .sort()
+    .forEach((key) => {
+      sorted[key] = normalizeObject((obj as Record<string, unknown>)[key])
+    })
+
+  return sorted
+}
+
 /**
  * Generate a stable hash for a query object
  * @param query - Query object to hash
  * @param idProp - ID property name
  * @returns Hash string
  */
 function hashQuery(query: Record<string, unknown>, idProp: string): string {
-  // Create deterministic string representation
-  const normalized = JSON.stringify(query, Object.keys(query).sort())
+  // Create deterministic string representation with deep key sorting
+  const normalized = JSON.stringify(normalizeObject(query))
   return createHash('sha256').update(`${normalized}:${idProp}`).digest('hex').slice(0, 16)
 }