[BUGFIX beta] Fix ember-metal/cache

In emberjs#13829, we introduced the ability to pass a custom key function for
the cache. However, we are incorrectly passing `(key, value)` to `set`
when the function expects `(obj, value)`.

This is causing problems for caches that uses a custom cache keying
function (such as Glimmer's compilation cache), because the get side
would handle the keying correctly, but the set side would compute the
key based on the already-computed key (i.e. `this.key(this.key(obj))`).

In Glimmer's compilation cache, this is causing the cache to always miss
because the `set` side would cache everything under the `"undefined"`
key.

packages/ember-metal/lib/cache.js

@@ -11,25 +11,12 @@ export default class Cache {
     this.store  = store || new DefaultStore();
   }
 
-  set(obj, value) {
-    if (this.limit > this.size) {
-      let key = this.key === undefined ? obj : this.key(obj);
-      this.size ++;
-      if (value === undefined) {
-        this.store.set(key, UNDEFINED);
-      } else {
-        this.store.set(key, value);
-      }
-    }
-    return value;
-  }
-
   get(obj) {
     let key = this.key === undefined ? obj : this.key(obj);
     let value = this.store.get(key);
     if (value === undefined) {
       this.misses ++;
-      value = this.set(key, this.func(obj));
+      value = this._set(key, this.func(obj));
     } else if (value === UNDEFINED) {
       this.hits ++;
       value = undefined;
@@ -41,6 +28,24 @@ export default class Cache {
     return value;
   }
 
+  set(obj, value) {
+    let key = this.key === undefined ? obj : this.key(obj);
+    return this._set(key, value);
+  }
+
+  _set(key, value) {
+    if (this.limit > this.size) {
+      this.size ++;
+      if (value === undefined) {
+        this.store.set(key, UNDEFINED);
+      } else {
+        this.store.set(key, value);
+      }
+    }
+
+    return value;
+  }
+
   purge() {
     this.store.clear();
     this.size   = 0;

packages/ember-metal/tests/cache_test.js

@@ -10,6 +10,20 @@ QUnit.test('basic', function() {
   equal(cache.get('foo'), 'FOO');
 });
 
+QUnit.test('explicit sets', function() {
+  let cache = new Cache(100, key => key.toUpperCase());
+
+  equal(cache.get('foo'), 'FOO');
+
+  equal(cache.set('foo', 'FOO!!!'), 'FOO!!!');
+
+  equal(cache.get('foo'), 'FOO!!!');
+
+  strictEqual(cache.set('foo', undefined), undefined);
+
+  strictEqual(cache.get('foo'), undefined);
+});
+
 QUnit.test('caches computation correctly', function() {
   let count = 0;
   let cache = new Cache(100, key => {
@@ -28,10 +42,41 @@ QUnit.test('caches computation correctly', function() {
   equal(count, 2);
 });
 
+QUnit.test('caches computation correctly with custom cache keys', function() {
+  let count = 0;
+  let cache = new Cache(
+    100,
+    obj => {
+      count++;
+      return obj.value.toUpperCase();
+    },
+    obj => obj.key
+  );
+
+  equal(count, 0);
+  cache.get({ key: 'foo', value: 'foo' });
+  equal(count, 1);
+  cache.get({ key: 'bar', value: 'bar' });
+  equal(count, 2);
+  cache.get({ key: 'bar', value: 'bar' });
+  equal(count, 2);
+  cache.get({ key: 'foo', value: 'foo' });
+  equal(count, 2);
+});
+
 QUnit.test('handles undefined value correctly', function() {
-  let cache = new Cache(100, (key) => { });
+  let count = 0;
+  let cache = new Cache(100, key => { count++; });
 
-  equal(cache.get('foo'), undefined);
+  equal(count, 0);
+  strictEqual(cache.get('foo'), undefined);
+  equal(count, 1);
+  strictEqual(cache.get('bar'), undefined);
+  equal(count, 2);
+  strictEqual(cache.get('bar'), undefined);
+  equal(count, 2);
+  strictEqual(cache.get('foo'), undefined);
+  equal(count, 2);
 });
 
 QUnit.test('continues working after reaching cache limit', function() {