fix: concurrent write on LRU cache (#200)

harttle · Mar 9, 2020 · 6de9338 · 6de9338
1 parent 06ad048
commit 6de9338
Show file tree

Hide file tree

Showing 3 changed files with 69 additions and 13 deletions.
diff --git a/src/cache/lru.ts b/src/cache/lru.ts
@@ -25,13 +25,17 @@ export class LRU<T> implements Cache<T> {
   }
 
   write (key: string, value: T) {
-    const node = new Node(key, value, this.head.next, this.head)
-    this.head.next.prev = node
-    this.head.next = node
+    if (this.cache[key]) {
+      this.cache[key].value = value
+    } else {
+      const node = new Node(key, value, this.head.next, this.head)
+      this.head.next.prev = node
+      this.head.next = node
 
-    this.cache[key] = node
-    this.size++
-    this.ensureLimit()
+      this.cache[key] = node
+      this.size++
+      this.ensureLimit()
+    }
   }
 
   read (key: string): T | undefined {

diff --git a/test/integration/liquid/cache.ts b/test/integration/liquid/cache.ts
@@ -64,7 +64,7 @@ describe('LiquidOptions#cache', function () {
       const y = await engine.renderFile('files/foo')
       expect(y).to.equal('FOO')
     })
-    it('should respect cache={} option', async function () {
+    it('should respect cache={read, write} option', async function () {
       let last: Template[] | undefined
       const engine = new Liquid({
         root: '/root/',
@@ -81,7 +81,7 @@ describe('LiquidOptions#cache', function () {
       expect(await engine.renderFile('files/bar')).to.equal('foo')
       expect(await engine.renderFile('files/coo')).to.equal('foo')
     })
-    it('should respect cache={} option (async)', async function () {
+    it('should respect cache={ async read, async write } option', async function () {
       const cached: { [key: string]: Template[] | undefined } = {}
       const engine = new Liquid({
         root: '/root/',
@@ -100,6 +100,26 @@ describe('LiquidOptions#cache', function () {
       mock({ '/root/files/coo.html': 'COO' })
       expect(await engine.renderFile('files/coo')).to.equal('coo')
     })
+    it('should handle concurrent cache read/write', async function () {
+      const engine = new Liquid({
+        root: '/root/',
+        extname: '.html',
+        cache: 1
+      })
+      mock({ '/root/files/foo.html': 'foo' })
+      mock({ '/root/files/bar.html': 'bar' })
+      mock({ '/root/files/coo.html': 'coo' })
+      const [foo1, foo2, bar, coo] = await Promise.all([
+        engine.renderFile('files/foo'),
+        engine.renderFile('files/foo'),
+        engine.renderFile('files/bar'),
+        engine.renderFile('files/coo')
+      ])
+      expect(foo1).to.equal('foo')
+      expect(foo2).to.equal('foo')
+      expect(bar).to.equal('bar')
+      expect(coo).to.equal('coo')
+    })
     it('should not cache not exist file', async function () {
       const engine = new Liquid({
         root: '/root/',
@@ -135,12 +155,12 @@ describe('LiquidOptions#cache', function () {
         cache: true
       })
       mock({ '/root/foo.html': 'foo' })
-      const x = engine.renderFileSync('foo')
-      expect(x).to.equal('foo')
+      mock({ '/root/bar.html': 'bar' })
 
+      expect(engine.renderFileSync('foo')).to.equal('foo')
+      expect(engine.renderFileSync('bar')).to.equal('bar')
       mock({ '/root/foo.html': 'bar' })
-      const y = engine.renderFileSync('foo')
-      expect(y).to.equal('foo')
+      expect(engine.renderFileSync('foo')).to.equal('foo')
     })
     it('should not cache not exist file', async function () {
       const engine = new Liquid({

diff --git a/test/unit/cache/lru.ts b/test/unit/cache/lru.ts
@@ -20,7 +20,31 @@ describe('LRU', () => {
     expect(lru.size).to.equal(0)
     expect(lru.read('foo')).to.be.undefined
   })
-  it('should remove lrc item when full(2)', () => {
+  it('should remove lrc item when full(limit=-1)', () => {
+    const lru = new LRU(-1)
+    lru.write('foo', 'FOO')
+    lru.write('bar', 'BAR')
+    expect(lru.size).to.equal(0)
+    expect(lru.read('foo')).to.be.undefined
+    expect(lru.read('bar')).to.be.undefined
+  })
+  it('should remove lrc item when full(limit=0)', () => {
+    const lru = new LRU(0)
+    lru.write('foo', 'FOO')
+    lru.write('bar', 'BAR')
+    expect(lru.size).to.equal(0)
+    expect(lru.read('foo')).to.be.undefined
+    expect(lru.read('bar')).to.be.undefined
+  })
+  it('should remove lrc item when full(limit=1)', () => {
+    const lru = new LRU(1)
+    lru.write('foo', 'FOO')
+    lru.write('bar', 'BAR')
+    expect(lru.size).to.equal(1)
+    expect(lru.read('foo')).to.be.undefined
+    expect(lru.read('bar')).to.equal('BAR')
+  })
+  it('should remove lrc item when full(limit=2)', () => {
     const lru = new LRU(2)
     expect(lru.size).to.equal(0)
     lru.write('foo', 'FOO')
@@ -33,4 +57,12 @@ describe('LRU', () => {
     expect(lru.read('bar')).to.equal('BAR')
     expect(lru.read('coo')).to.equal('COO')
   })
+  it('should overwrite item the with same key', () => {
+    const lru = new LRU(2)
+    lru.write('foo', 'FOO')
+    expect(lru.size).to.equal(1)
+    lru.write('foo', 'BAR')
+    expect(lru.size).to.equal(1)
+    expect(lru.read('foo')).to.equal('BAR')
+  })
 })