fix: sync getBulk correctly decodes mixed hashed and plain keys

evcache-client/test/com/netflix/evcache/test/EVCacheTestDI.java

@@ -449,13 +449,12 @@ private void testWithMixedKeysAndCustomTranscoder() throws Exception {
 
         // async bulk get
         for (int op : new int[]{0, 1}) {
-            Map<String, Movie> results = new HashMap<>();
+            Map<String, Movie> results;
             if (op == 0) {
                 CompletableFuture<Map<String, Movie>> future = evCache.getAsyncBulk(kv.keySet().toArray(new String[0]));
                 results = future.get(10000, TimeUnit.MILLISECONDS);
-            // } else {
-                // TODO: getBulk api is known to be broken for un-hashed keys not decoding correctly when request contains both hashed and unhashed keys
-                // results = evCache.getBulk(kv.keySet().toArray(new String[0]));
+            } else {
+                results = evCache.getBulk(kv.keySet().toArray(new String[0]));
             }
 
             for (Map.Entry<String, Movie> result : results.entrySet()) {

evcache-core/src/main/java/com/netflix/evcache/EVCacheImpl.java

@@ -1969,60 +1969,38 @@ private <T>  Map<EVCacheKey, T> buildKeyValueResult(Map<String, T> objMap,
 
     private <T> Map<EVCacheKey, T> getBulkData(EVCacheClient client, Collection<EVCacheKey> evcacheKeys, Transcoder<T> tc, boolean throwException, boolean hasZF) throws Exception {
         try {
-            boolean hasHashedKey = false;
-            final Map<String, EVCacheKey> keyMap = new HashMap<>(evcacheKeys.size() * 2);
-            for(EVCacheKey evcKey : evcacheKeys) {
-                String key = evcKey.getCanonicalKey(client.isDuetClient());
-                String hashKey = evcKey.getHashKey(client.isDuetClient(), client.getHashingAlgorithm(), client.shouldEncodeHashKey(), client.getMaxDigestBytes(), client.getMaxHashLength(), client.getBaseEncoder());
-                if(hashKey != null) {
-                    if (log.isDebugEnabled() && shouldLog()) log.debug("APP " + _appName + ", key [" + key + "], has been hashed [" + hashKey + "]");
-                    key = hashKey;
-                    hasHashedKey = true;
-                }
-                keyMap.put(key, evcKey);
-            }
-            if(hasHashedKey) {
-                final Map<String, Object> objMap = client.getBulk(keyMap.keySet(), evcacheValueTranscoder, throwException, hasZF);
-                final Map<EVCacheKey, T> retMap = new HashMap<>((int) (objMap.size() / 0.75) + 1);
-                for (Map.Entry<String, Object> i : objMap.entrySet()) {
-                    final Object obj = i.getValue();
-                    if(obj instanceof EVCacheValue) {
-                        if (log.isDebugEnabled() && shouldLog()) log.debug("APP " + _appName + ", The value for key [" + i.getKey() + "] is EVCache Value");
-                        final EVCacheValue val = (EVCacheValue)obj;
-                        final CachedData cd = new CachedData(val.getFlags(), val.getValue(), CachedData.MAX_SIZE);
-                        final T tVal;
-                        if(tc == null) {
-                            tVal = (T)client.getTranscoder().decode(cd);
-                        } else {
-                            tVal = tc.decode(cd);
-                        }
-                        final EVCacheKey evcKey = keyMap.get(i.getKey());
-                        if(evcKey.getCanonicalKey(client.isDuetClient()).equals(val.getKey())) {
-                            if (log.isDebugEnabled() && shouldLog()) log.debug("APP " + _appName + ", key [" + i.getKey() + "] EVCacheKey " + evcKey);
-                            retMap.put(evcKey, tVal);
-                        } else {
-                            if (log.isDebugEnabled() && shouldLog()) log.debug("CACHE COLLISION : APP " + _appName + ", key [" + i.getKey() + "] EVCacheKey " + evcKey);
-                            incrementFailure(EVCacheMetricsFactory.KEY_HASH_COLLISION, Call.BULK.name(), EVCacheMetricsFactory.READ);
-                        }
-                    } else {
-                        final EVCacheKey evcKey = keyMap.get(i.getKey());
-                        if (log.isDebugEnabled() && shouldLog()) log.debug("APP " + _appName + ", key [" + i.getKey() + "] EVCacheKey " + evcKey);
-                        retMap.put(evcKey, (T)obj);
-                    }
+            final KeyMapDto keyMapDto = buildKeyMap(client, evcacheKeys);
+            final Set<String> plainKeys = keyMapDto.getPlainKeysMap().keySet();
+            final Set<String> hashedKeys = keyMapDto.getHashedKeysMap().keySet();
+
+            // Preserve chunking behavior for plain-only requests: route through the existing single-transcoder
+            // path, which is the only one that honors enableChunking. Chunked apps do not use hashed keys.
+            if (hashedKeys.isEmpty()) {
+                if (tc == null && _transcoder != null) tc = (Transcoder<T>) _transcoder;
+                final Map<String, T> objMap = client.getBulk(plainKeys, tc, throwException, hasZF);
+                return buildKeyValueResult(objMap, keyMapDto);
+            }
+
+            final BiPredicate<String, String> collisionChecker = (hashedKey, decodedKey) -> {
+                final EVCacheKey evcKey = keyMapDto.getHashedKeysMap().get(hashedKey);
+                if (evcKey.getCanonicalKey(client.isDuetClient()).equals(decodedKey)) {
+                    if (log.isDebugEnabled() && shouldLog())
+                        log.debug("APP " + _appName + ", key [" + hashedKey + "] EVCacheKey " + evcKey);
+                } else {
+                    if (log.isDebugEnabled() && shouldLog())
+                        log.debug("CACHE COLLISION : APP " + _appName + ", key [" + hashedKey + "] EVCacheKey " + evcKey + " with decodedKey [" + decodedKey + "]");
+                    incrementFailure(EVCacheMetricsFactory.KEY_HASH_COLLISION, Call.BULK.name(), EVCacheMetricsFactory.READ);
+                    return true;
                 }
-                return retMap;
+                return false;
+            };
 
-            } else {
-                if(tc == null && _transcoder != null) tc = (Transcoder<T>)_transcoder;
-                final Map<String, T> objMap = client.getBulk(keyMap.keySet(), tc, throwException, hasZF);
-                final Map<EVCacheKey, T> retMap = new HashMap<EVCacheKey, T>((int)(objMap.size()/0.75) + 1);
-                for (Map.Entry<String, T> i : objMap.entrySet()) {
-                    final EVCacheKey evcKey = keyMap.get(i.getKey());
-                    if (log.isDebugEnabled() && shouldLog()) log.debug("APP " + _appName + ", key [" + i.getKey() + "] EVCacheKey " + evcKey);
-                    retMap.put(evcKey, i.getValue());
-                }
-                return retMap;
-            }
+            final Transcoder<T> valueTranscoder = (tc == null) ? ((_transcoder == null) ? (Transcoder<T>) client.getTranscoder() : (Transcoder<T>) _transcoder) : tc;
+            if (log.isDebugEnabled() && shouldLog())
+                log.debug("fetching bulk data with set of keys containing hashed key(s) {} ", evcacheKeys);
+
+            final Map<String, T> objMap = client.getBulk(plainKeys, hashedKeys, valueTranscoder, evcacheValueTranscoder, _appName, shouldLog(), collisionChecker, throwException, hasZF);
+            return buildKeyValueResult(objMap, keyMapDto);
         } catch (Exception ex) {
             if (log.isDebugEnabled() && shouldLog()) log.debug("Exception while getBulk data for APP " + _appName + ", key : " + evcacheKeys, ex);
             if (!throwException || hasZF) return null;

evcache-core/src/main/java/com/netflix/evcache/pool/EVCacheClient.java

@@ -998,6 +998,35 @@ public <T> Map<String, T> getBulk(Collection<String> canonicalKeys, Transcoder<T
         return returnVal;
     }
 
+    /**
+     * Sync sibling of {@link #getAsyncBulk(Collection, Set, Transcoder, EVCacheTranscoder, String, boolean, BiPredicate)}.
+     * Routes through the same mixed-key-aware {@link EVCacheMemcachedClient#asyncGetBulk} entry point and blocks on
+     * {@code .getSome(...)} so hashed and plain keys in one request are each decoded with the correct transcoder
+     * (two-step for hashed, one-step for plain). Does not support chunking; chunked apps must keep using
+     * {@link #getBulk(Collection, Transcoder, boolean, boolean)}.
+     */
+    public <T> Map<String, T> getBulk(Collection<String> plainKeys, Set<String> hashedKeys,
+                                      Transcoder<T> valueTranscoder, EVCacheTranscoder evcacheValueTranscoder,
+                                      String appName, boolean shouldLog, BiPredicate<String, String> collisionChecker,
+                                      boolean _throwException, boolean hasZF) throws Exception {
+        try {
+            if (valueTranscoder == null) valueTranscoder = (Transcoder<T>) getTranscoder();
+            final BiPredicate<MemcachedNode, String> validator = (node, key) -> {
+                NodeValidationResult result = validateNodeForRead(node, Call.BULK, 2 * maxReadQueueSize.get());
+                if (result != NodeValidationResult.OK) {
+                    return false;
+                }
+                return true;
+            };
+            return evcacheMemcachedClient
+                    .asyncGetBulk(plainKeys, hashedKeys, valueTranscoder, evcacheValueTranscoder, validator, appName, shouldLog, collisionChecker)
+                    .getSome(bulkReadTimeout.get(), TimeUnit.MILLISECONDS, _throwException, hasZF);
+        } catch (Exception e) {
+            if (_throwException) throw e;
+            return Collections.<String, T> emptyMap();
+        }
+    }
+
     /**
      * @Deprecated This method does NOT support a mix of plain and hashed keys in {@code keys}. All keys are
      * decoded exactly using the given transcoder (note that hashed keys require two step decoding).