RedisClient: Use sets in target URL index

This makes the underlying logic a lot easier to work with, especially when writing the migration script to generate the target URL index from an existing database.
mbland · Apr 24, 2018 · 6f18f2d · 6f18f2d
1 parent 4514690
commit 6f18f2d
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 13 deletions.
diff --git a/lib/redis/index.js b/lib/redis/index.js
@@ -68,8 +68,8 @@ function getShortLinksFromTargetLinks(redisClient, targetKeys) {
 
 function getShortLinksFromTargetLink(redisClient, targetLink) {
   return new Promise((resolve, reject) => {
-    redisClient.impl.lrange(targetLink, 0, -1, (err, shortLinks) => {
-      return err ? reject(err) : resolve(shortLinks)
+    redisClient.impl.smembers(targetLink, (err, shortLinks) => {
+      return err ? reject(err) : resolve(shortLinks.sort())
     })
   })
 }

diff --git a/lib/redis/target-indexer.js b/lib/redis/target-indexer.js
@@ -1,7 +1,6 @@
 'use strict'
 
 var Keys = require('./keys')
-var ListHelper = require('./list-helper')
 var PromiseHelper = require('./promise-helper')
 
 module.exports = class TargetIndexer {
@@ -11,7 +10,7 @@ module.exports = class TargetIndexer {
 
   addLink(link, linkInfo) {
     return PromiseHelper.do(done => {
-      this.impl.lpush(Keys.targetIndex(linkInfo.target), link, done)
+      this.impl.sadd(Keys.targetIndex(linkInfo.target), link, done)
     })
   }
 
@@ -20,14 +19,14 @@ module.exports = class TargetIndexer {
   }
 
   removeLink(link, linkInfo) {
-    return new ListHelper(this.impl)
-      .removeItem(Keys.targetIndex(linkInfo.target), link)
+    return PromiseHelper.do(done => {
+      this.impl.srem(Keys.targetIndex(linkInfo.target), link, done)
+    })
   }
 
   getLinksToTarget(target) {
-    var getLinks = done => {
-      this.impl.lrange(Keys.targetIndex(target), 0, -1, done)
-    }
-    return PromiseHelper.do(getLinks).then(links => links.sort())
+    return PromiseHelper
+      .do(done => this.impl.smembers(Keys.targetIndex(target), done))
+      .then(links => links.sort())
   }
 }
diff --git a/tests/server/redis-client-test.js b/tests/server/redis-client-test.js
@@ -421,14 +421,14 @@ describe('RedisClient', function() {
     })
 
     it('resolves to an Error when reindexing a link fails', () => {
-      stubClientImplMethod('lrem').callsFake((...args) => {
+      stubClientImplMethod('srem').callsFake((...args) => {
         var cb = args.pop()
         cb(new Error('forced error for ' + args.join(' ')))
       })
 
       return redisClient.reindexLink('/foo', { target: LINK_TARGET }, {})
         .should.be.rejectedWith(Error,
-          'forced error for target:' + LINK_TARGET + ' 1 /foo')
+          'forced error for target:' + LINK_TARGET + ' /foo')
     })
   })
 
@@ -530,7 +530,7 @@ describe('RedisClient', function() {
         return redisClient.searchTargetLinks()
       }).should.be.fulfilled.then(function(link) {
         link.should.eql({
-          'https://mike-bland.com/': ['/baz', '/bar', '/foo'],
+          'https://mike-bland.com/': ['/bar', '/baz', '/foo'],
           'https://akash.com': ['/test']
         })
       })