implement handling multiple repos

Signed-off-by: Jakub Sokołowski <jakub@status.im>

README.md

@@ -64,7 +64,7 @@ There are few environment variables you can set:
 * `DB_PATH` - Path where the [LokiJS](http://lokijs.org/#/) DB file is stored. (Default: `/tmp/builds.db`)
 * `GH_TOKEN` - Required for GitHub API access.
 * `GH_REPO_OWNER` - Name of owner of repo to manage.
-* `GH_REPO_NAME` - Name of GitHub repo to manage.
+* `GH_REPO_NAMES` - Whitelist of names of GitHub repos to manage. (Empty means all)
 
 # Building
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "github-comment-manager",
-  "version": "0.1.1",
+  "version": "0.2.0",
   "description": "Minimal API for managing GitHub comments for CIclicks.",
   "repository": "https://github.com/status-im/github-comment-manager",
   "main": "index.js",

src/app.js

@@ -6,7 +6,7 @@ const JsonError = require('koa-json-error')
 const JoiRouter = require('koa-joi-router')
 const BodyParser = require('koa-bodyparser')
 
-const App = (ghc) => {
+const App = ({ghc, schema}) => {
   const app = new Koa()
   const router = new JoiRouter()
 
@@ -24,35 +24,61 @@ const App = (ghc) => {
     ctx.body = 'OK'
   })
 
+  /* TEMPORARY fix to keep backwards compatibility */
   router.route({
     method: 'post',
     path: '/builds/:pr',
     validate: {
       type: 'json',
-      body: ghc.db.schema,
+      body: schema,
     },
     handler: async (ctx) => {
-      /* save the build */
+      await ghc.db.addBuild({
-      await ghc.db.addBuild(ctx.params.pr, ctx.request.body)
+        repo: 'status-react', 
-      /* post or update the comment */
+        pr: ctx.params.pr,
-      await ghc.update(ctx.params.pr)
+        build: ctx.request.body,
+      })
+      await ghc.update({
+        repo: 'status-react',
+        pr: ctx.params.pr,
+      })
       ctx.status = 201
       ctx.body = {status:'ok'}
     }
   })
   
-  router.post('/builds/:pr/refresh', async (ctx) => {
+  /* store build and post/update the comment */
-    /* just re-render the comment */
+  router.route({
-    await ghc.update(ctx.params.pr)
+    method: 'post',
+    path: '/builds/:repo/:pr',
+    validate: {
+      type: 'json',
+      body: schema,
+    },
+    handler: async (ctx) => {
+      await ghc.db.addBuild({
+        ...ctx.params, build: ctx.request.body
+      })
+      await ghc.update(ctx.params)
+      ctx.status = 201
+      ctx.body = {status:'ok'}
+    }
+  })
+  
+  /* just re-render the comment */
+  router.post('/builds/:repo/:pr/refresh', async (ctx) => {
+    await ghc.update(ctx.params)
     ctx.status = 201
     ctx.body = {status:'ok'}
   })
 
-  router.get('/builds/:pr', async (ctx) => {
+  /* list builds for repo+pr */
-    const builds = await ghc.db.getBuilds(ctx.params.pr)
+  router.get('/builds/:repo/:pr', async (ctx) => {
+    const builds = await ghc.db.getBuilds(ctx.params)
     ctx.body = {count: builds.length, builds}
   })
 
+  /* list all managed comments */
   router.get('/comments', async (ctx) => {
     const comments = await ghc.db.getComments()
     ctx.body = {count: comments.length, comments}

src/builds.js

@@ -2,11 +2,9 @@ const log = require('loglevel')
 const Joi = require('joi')
 const Loki = require('lokijs')
 const AwaitLock = require('await-lock')
-const schema = require('./schema')
 
 class Builds {
   constructor(path, interval) {
-    this.schema = schema
     this.lock = new AwaitLock()
     this.db = new Loki(path, {
       autoload: true,
@@ -29,10 +27,6 @@ class Builds {
     this.db.on('close', () => this.save())
   }
 
-  validate (build) {
-    return Joi.validate(build, this.schema)
-  }
-
   async save () {
     this.db.saveDatabase((err) => {
       if (err) { console.error('error saving', err) }
@@ -59,9 +53,9 @@ class Builds {
     return [].concat.apply([], bc)
   }
 
-  async getBuilds (pr) {
+  async getBuilds (query) {
     let builds = await this.builds.chain()
-      .find({pr})
+      .find(query)
       .compoundsort(['$loki'])
       .data()
     /* sort groups of builds for commit based on $loki */
@@ -73,33 +67,33 @@ class Builds {
     })
   }
 
-  async addBuild (pr, build) {
+  async addBuild ({repo, pr, build}) {
     log.info(`Storing build for PR-${pr}: #${build.id} for ${build.platform}`)
     return await this.builds.insert({pr, ...build})
   }
 
-  async addComment (pr, comment_id) {
+  async addComment ({repo, pr, comment_id}) {
     await this.lock.acquireAsync()
     try {
       log.info(`Storing comment for PR-${pr}: ${comment_id}`)
-      return await this.comments.insert({pr, comment_id})
+      return await this.comments.insert({repo, pr, comment_id})
     } finally {
       this.lock.release()
     }
   }
 
-  async getCommentID (pr) {
+  async getCommentID (repo, pr) {
     await this.lock.acquireAsync()
     try {
-      const rval = await this.comments.findOne({pr: pr})
+      const rval = await this.comments.findOne({repo, pr})
       return rval ? rval.comment_id : null
     } finally {
       this.lock.release()
     }
   }
 
-  async getComments (pr) {
+  async getComments () {
-    const comments = await this.comments.chain().simplesort('pr').data();
+    const comments = await this.comments.chain().simplesort('pr').data()
     /* strip the loki attributes */
     return comments.map((c) => {
       const {$loki, meta, ...comment} = c

src/comments.js

@@ -46,10 +46,9 @@ const extractArchiveBuilds = (builds) => {
 }
 
 class Comments {
-  constructor(client, owner, repo, builds) {
+  constructor({client, owner, builds}) {
     this.gh = client
     this.db = builds
-    this.repo = repo   /* name of repo to query */
     this.owner = owner /* name of user who makes the comments */
     /* add helper for formatting dates */
     Handlebars.registerHelper('date', dateHelper)
@@ -67,8 +66,8 @@ class Comments {
     this.template = Handlebars.compile(template.main);
   }
 
-  async renderComment (pr) {
+  async renderComment ({repo, pr}) {
-    const builds = await this.db.getBuilds(pr)
+    const builds = await this.db.getBuilds({repo, pr})
     if (builds.length == 0) {
       throw Error('No builds exist for this PR')
     }
@@ -77,38 +76,38 @@ class Comments {
     return this.template({visible, archived})
   }
 
-  async postComment (pr) {
+  async postComment ({repo, pr}) {
     log.info(`Creating comment in PR-${pr}`)
-    const body = await this.renderComment(pr)
+    const body = await this.renderComment({repo, pr})
     const rval = await this.gh.issues.createComment({
       owner: this.owner,
-      repo: this.repo,
+      repo: repo,
       number: pr,
       body,
     })
     return rval.data.id
   }
 
-  async updateComment (pr, comment_id) {
+  async updateComment ({repo, pr, comment_id}) {
     log.info(`Updating comment in PR-${pr}`)
-    const body = await this.renderComment(pr)
+    const body = await this.renderComment({repo, pr})
     const rval = await this.gh.issues.updateComment({
       owner: this.owner,
-      repo: this.repo,
+      repo: repo,
       comment_id,
       body,
     })
     return rval.data.id
   }
 
-  async update (pr) {
+  async update ({repo, pr}) {
     /* check if comment was already posted */
-    let id = await this.db.getCommentID(pr)
+    let comment_id = await this.db.getCommentID(repo, pr)
-    if (id) {
+    if (comment_id) {
-      await this.updateComment(pr, id)
+      await this.updateComment({repo, pr, comment_id})
     } else {
-      id = await this.postComment(pr)
+      comment_id = await this.postComment({repo, pr})
-      await this.db.addComment(pr, id)
+      await this.db.addComment({repo, pr, comment_id})
     }
   }
 }

src/schema.js

@@ -1,13 +1,17 @@
 const Joi = require('joi')
  
-const schema = Joi.object().keys({
+/* whitelisted repos are controlled by env variables in server.js */
-  id: Joi.alternatives().try(Joi.number().positive(), Joi.string()).required(),
+const genSchema = (REPOS_WHITELIST) => (
-  commit: Joi.string().regex(/^[a-zA-Z0-9]{6,40}$/).required(),
+  Joi.object().keys({
-  success: Joi.boolean().required(),
+    id: Joi.alternatives().try(Joi.number().positive(), Joi.string()).required(),
-  platform: Joi.string().max(20).required(),
+    commit: Joi.string().regex(/^[a-zA-Z0-9]{6,40}$/).required(),
-  duration: Joi.string().max(20).required(),
+    repo: Joi.string().max(30).required().valid(REPOS_WHITELIST),
-  url: Joi.string().uri().required(),
+    success: Joi.boolean().required(),
-  pkg_url: Joi.string().uri().allow(null),
+    platform: Joi.string().max(20).required(),
-})
+    duration: Joi.string().max(20).required(),
+    url: Joi.string().uri().required(),
+    pkg_url: Joi.string().uri().allow(null),
+  })
+)
 
-module.exports = schema
+module.exports = genSchema

src/server.js

@@ -5,13 +5,14 @@ const Octokit = require('@octokit/rest')
 const App = require('./app')
 const Builds = require('./builds')
 const Comments = require('./comments')
+const Schema = require('./schema')
 
 /* DEFAULTS */
 const LOG_LEVEL        = process.env.LOG_LEVEL        || 'INFO'
 const LISTEN_PORT      = process.env.LISTEN_PORT      || 8000
 const GH_TOKEN         = process.env.GH_TOKEN         || null
 const GH_REPO_OWNER    = process.env.GH_REPO_OWNER    || 'status-im'
-const GH_REPO_NAME     = process.env.GH_REPO_NAME     || 'status-react'
+const GH_REPO_NAMES    = process.env.GH_REPO_NAMES    || []
 const DB_PATH          = process.env.DB_PATH          || '/tmp/builds.db'
 const DB_SAVE_INTERVAL = process.env.DB_SAVE_INTERVAL || 5000
 
@@ -25,8 +26,15 @@ const builds = new Builds(DB_PATH, DB_SAVE_INTERVAL)
 const gh = new Octokit()
 gh.authenticate({type: 'token', token: GH_TOKEN})
 
-const ghc = new Comments(gh, GH_REPO_OWNER, GH_REPO_NAME, builds)
+/* set valid repo names */
-const app = App(ghc, builds)
+const schema = Schema(GH_REPO_NAMES)
+
+const ghc = new Comments({
+  client: gh,
+  owner: GH_REPO_OWNER,
+  builds: builds,
+})
+const app = App({ghc, schema})
 
 app.use(Logger())
 

test/app.js

@@ -15,7 +15,7 @@ describe('App', () => {
     ghc.db = sinon.createStubInstance(Builds, {
       getComments: sample.COMMENTS,
     }),
-    app = App(ghc)
+    app = App({ghc})
   })
 
   describe('GET /health', () => {
@@ -38,28 +38,36 @@ describe('App', () => {
     })
   })
 
-  describe('POST /builds/:pr', () => {
+  describe('POST /builds/:repo/:pr', () => {
     it('should store the POSTed build', async () => {
       const resp = await request(app.callback())
-        .post('/builds/PR-1')
+        .post('/builds/REPO-1/PR-1')
         .send(sample.BUILD)
       expect(resp.body).to.eql({status:'ok'})
       expect(resp.status).to.eq(201)
-      expect(ghc.db.addBuild).calledOnceWith('PR-1', sample.BUILD)
+      expect(ghc.db.addBuild).calledOnceWith({
-      expect(ghc.update).calledOnceWith('PR-1')
+        repo: 'REPO-1', pr: 'PR-1', build: sample.BUILD,
+      })
+      expect(ghc.update).calledOnceWith({
+        repo: 'REPO-1', pr: 'PR-1'
+      })
     })
   })
 
 
-  describe('POST /builds/:pr/refresh', () => {
+  describe('POST /builds/:repo/:pr/refresh', () => {
     it('should update github comment', async () => {
       const resp = await request(app.callback())
-        .post('/builds/PR-1/refresh')
+        .post('/builds/REPO-1/PR-1/refresh')
         .send(sample.BUILD)
       expect(resp.body).to.eql({status:'ok'})
       expect(resp.status).to.eq(201)
-      expect(ghc.db.addBuild).not.calledOnceWith('PR-1', sample.BUILD)
+      expect(ghc.db.addBuild).not.calledOnceWith({
-      expect(ghc.update).calledOnceWith('PR-1')
+        repo: 'REPO-1', pr: 'PR-1', build: sample.BUILD
+      })
+      expect(ghc.update).calledOnceWith({
+        repo: 'REPO-1', pr: 'PR-1',
+      })
     })
   })
 })

test/builds.js

@@ -36,7 +36,7 @@ describe('Builds', () => {
       /* need to add the builds before they can be sorted */
       for (let i=0; i<BUILDS.length; i++) {
         let b = BUILDS[i]
-        await builds.addBuild('PR-1', b)
+        await builds.addBuild({repo: 'REPO-1', pr: 'PR-1', build: b})
         /* verify the build was added */
         let rval = await builds.builds.findOne({id: b.id, platform: b.platform})
         expect(rval.commit).to.equal(BUILDS[i].commit)
@@ -44,10 +44,10 @@ describe('Builds', () => {
     })
 
     it('should sort by commits and ids', async () => {
-      let rval = await builds.getBuilds('PR-1')
+      let rval = await builds.getBuilds('REPO-1', 'PR-1')
       /* remove fields we don't care about for easier comparison */
       rval = rval.map((b) => {
-        const { pr, success, duration, url, pkg_url, meta, ...build } = b
+        const { pr, repo, success, duration, url, pkg_url, meta, ...build } = b
         return build
       })
       expect(rval).to.deep.equal([

test/comments.js

@@ -54,7 +54,12 @@ describe('Comments', () => {
     builds = sinon.createStubInstance(Builds, {
       getBuilds: sample.BUILDS.slice(0, 2),
     })
-    comments = new Comments(client, 'owner', 'repo', builds)
+    comments = new Comments({
+      client: client,
+      owner: 'owner',
+      repos: ['repo'],
+      builds: builds
+    })
   })
   
   describe('renderComment', () => {
@@ -77,26 +82,30 @@ describe('Comments', () => {
 
   describe('postComment', () => {
     it('should create a new comment', async () => {
-      let id = await comments.postComment('PR-ID')
+      let id = await comments.postComment({
+        repo: 'REPO-1', pr: 'PR-ID',
+      })
       expect(id).to.eq('ISSUE-ID')
       expect(client.issues.createComment).calledOnceWith({
         body: sinon.match.any,
-        number: 'PR-ID',
         owner: 'owner',
-        repo: 'repo',
+        number: 'PR-ID',
+        repo: 'REPO-1',
       })
     })
   })
 
   describe('updateComment', () => {
     it('should update existing comment', async () => {
-      let id = await comments.updateComment('PR-ID', 'COMMENT-ID')
+      let id = await comments.updateComment({
+        repo: 'REPO-1', pr: 'PR-ID', comment_id: 'COMMENT-ID',
+      })
       expect(id).to.eq('ISSUE-ID')
       expect(client.issues.updateComment).calledOnceWith({
         body: sinon.match.any,
-        comment_id: 'COMMENT-ID',
         owner: 'owner',
-        repo: 'repo',
+        comment_id: 'COMMENT-ID',
+        repo: 'REPO-1',
       })
     })
   })

test/sample.js

@@ -2,6 +2,7 @@
 const BUILD = {
   id: 'ID-1',
   commit: 'abcd1234',
+  repo: 'REPO-1',
   success: true,
   platform: 'PLATFORM-1',
   duration: 'DURATION-1',
@@ -12,6 +13,7 @@ const BUILD = {
 const getBuild = (idx) => ({
   id: `ID-${idx}`,
   commit: `COMMIT-${Math.floor(idx/4)}`,
+  repo: `REPO-${Math.floor(idx/8)}`,
   success: (idx%3) ? true : false,
   platform: `PLATFORM-${idx}`,
   duration: `DURATION-${idx} 12 sec`,

test/schema.js

@@ -5,64 +5,82 @@ const Joi = require('joi')
 const sample = require('./sample')
 const Schema = require('../src/schema')
 
-let build
+let build, schema
 
 describe('Schema', () => {
   beforeEach(() => {
     /* refresh for every test */
     build = Object.assign({}, sample.BUILD)
+    schema = Schema(['REPO-1'])
   })
   
   describe('id', () => {
     it('can be a string', async () => {
-      let rval = await Joi.validate(build, Schema)
+      let rval = await Joi.validate(build, schema)
       expect(rval).to.eql(build)
     })
 
     it('can be a number', async () => {
       build.id = 123
-      let rval = await Joi.validate(build, Schema)
+      let rval = await Joi.validate(build, schema)
       expect(rval).to.eql(build)
     })
 
     it('can\'t be null', () => {
       build.id = null
-      expect(Joi.validate(build, Schema)).rejectedWith('"id" must be a number, "id" must be a string')
+      expect(Joi.validate(build, schema)).rejectedWith('"id" must be a number, "id" must be a string')
     })
   })
 
   describe('commit', () => {
     it('has to be a commit', async () => {
-      let rval = await Joi.validate(build, Schema)
+      let rval = await Joi.validate(build, schema)
       expect(rval).to.eql(build)
     })
 
     it('can\'t be a null', () => {
       build.commit = null
-      expect(Joi.validate(build, Schema)).rejectedWith('"commit" must be a string')
+      expect(Joi.validate(build, schema)).rejectedWith('"commit" must be a string')
     })
 
     it('can\'t be a number', () => {
       build.commit = 1
-      expect(Joi.validate(build, Schema)).rejectedWith('"commit" must be a string')
+      expect(Joi.validate(build, schema)).rejectedWith('"commit" must be a string')
+    })
+  })
+
+  describe('repo', () => {
+    it('has to be a repo', async () => {
+      let rval = await Joi.validate(build, schema)
+      expect(rval).to.eql(build)
+    })
+
+    it('can\'t be a null', () => {
+      build.repo = null
+      expect(Joi.validate(build, schema)).rejectedWith('"repo" must be a string')
+    })
+
+    it('has to be on whitelist', () => {
+      build.repo = 'REPO-WRONG'
+      expect(Joi.validate(build, schema)).rejectedWith('"repo" must be one of [REPO-1]')
     })
   })
 
   describe('pkg_url', () => {
     it('has to be a URL', async () => {
-      let rval = await Joi.validate(build, Schema)
+      let rval = await Joi.validate(build, schema)
       expect(rval).to.eql(build)
     })
 
     it('can be a null', async () => {
       build.pkg_url = null
-      let rval = await Joi.validate(build, Schema)
+      let rval = await Joi.validate(build, schema)
       expect(rval).to.eql(build)
     })
 
     it('can\'t be a number', () => {
       build.pkg_url = 1
-      expect(Joi.validate(build, Schema)).rejectedWith('"pkg_url" must be a string')
+      expect(Joi.validate(build, schema)).rejectedWith('"pkg_url" must be a string')
     })
   })
 })