Pull repositories automatically (#8)

* Big refactoring, replace `fetch` by `pull` about repositories since it
was confusing

* Reduce pause delay

* Minor adjustments

* Fix ugly and unreliable hardcoded delay in the e2e tests

* Add database migration to rename fetch -> pull

client/src/actions.js

@@ -89,23 +89,6 @@ const receiveDeleteRepoError = error => ({
 
 
 
-const requestPullRepo = name => dispatch => {
-    dispatch({type: 'wsRequestPullRepo', name})
-    wsSend({type: 'pullRepo', name})
-}
-
-const receivePullRepo = name => ({
-    type: 'wsReceivePullRepo',
-    name,
-})
-
-const receivePullRepoError = error => ({
-    type: 'wsReceivePullRepoError',
-    error,
-})
-
-
-
 const requestSearch = ({repoName, query}) => dispatch => {
     if (query) {
         dispatch({type: 'wsRequestSearch', repoName, query})
@@ -160,10 +143,6 @@ export default {
     receiveDeleteRepo,
     receiveDeleteRepoError,
 
-    requestPullRepo,
-    receivePullRepo,
-    receivePullRepoError,
-
     requestLogout,
 
     requestSearch,

client/src/index.scss

@@ -72,7 +72,7 @@ html, body, #react-root, main {
             color: $gray !important;
         }
 
-        &--fetchFailed {
+        &--pullFailed {
             @include error;
         }
     }

client/src/repo-info.js

@@ -22,8 +22,8 @@ export const NotFound = f(({name}) =>
 
 export const getRepoStatus = repo => [
     !repo.cloned && 'not cloned yet',
-    repo.beingFetched && 'being fetched',
-    repo.fetchFailed && 'last fetch failed',
+    repo.beingPulled && 'being pulled',
+    repo.pullFailed && 'last pull failed',
 ].filter(v => v).join(', ')
 
 class RepoInfoPageV extends React.Component {
@@ -37,11 +37,6 @@ class RepoInfoPageV extends React.Component {
         return dispatch(actions.requestDeleteRepo(repoName))
     }
 
-    pull = () => {
-        const {dispatch, repoName} = this.props
-        return dispatch(actions.requestPullRepo(repoName))
-    }
-
     componentWillReceiveProps(nextProps) {
         if (nextProps.deleteRepo.name == nextProps.repoName) {
             nextProps.router.navigate('root')
@@ -80,11 +75,6 @@ class RepoInfoPageV extends React.Component {
                 'search',
             ),
 
-            loggedIn && !repo.beingFetched && Button(
-                {onClick: this.pull},
-                repo.cloned ? 'pull' : 'clone again',
-            ),
-
             loggedIn && !deleteRepo.loading && Button(
                 {onClick: this.remove},
                 'delete',

client/src/repos.js

@@ -17,7 +17,7 @@ const RepoLink = f(({repo}) =>
             className: classnames(
                 'RepoLink',
                 !repo.cloned && 'RepoLink--notCloned',
-                repo.fetchFailed && 'RepoLink--fetchFailed',
+                repo.pullFailed && 'RepoLink--pullFailed',
             ),
             routeName: repo.cloned ? 'repositorySearch' : 'repositoryInfo',
             routeParams: {name: repo.name},

server/src/clones.js

@@ -1,5 +1,6 @@
 // Functions to manage cloned Git repositories
 
+const EventEmitter = require('events')
 const assert = require('assert')
 const {promisify} = require('util')
 const fs = require('fs')
@@ -57,14 +58,37 @@ const execGit = async (args, options) => {
     }
 }
 
-class Clones {
+// Events:
+//   - `pullStart` is emitted at the moment when a repo begins to
+//     be pulled (or cloned)
+//
+//   - `pullEnd` is emitted at the moment when a pull (or clone)
+//     ends (successfully or not).
+//
+// Given a repository A, `pullEnd` and `pullStart` are emitted just
+// after the return value of `clones.isBeingPulled(A)` has changed.
+class Clones extends EventEmitter {
     constructor(baseDirName) {
+        super()
+
         this._basePath = path.join(__dirname, '..', 'var', baseDirName)
-        this._beingFetched = new Set()
+        this._beingPulled = new Set()
+    }
+
+    isBeingPulled(name) {
+        return this._beingPulled.has(name)
     }
 
-    isBeingFetched(name) {
-        return this._beingFetched.has(name)
+    _emitPullStart(name) {
+        assert(!this.isBeingPulled(name))
+        this._beingPulled.add(name)
+        this.emit('pullStart', {repoName: name})
+    }
+
+    _emitPullEnd(name) {
+        assert(this.isBeingPulled(name))
+        this._beingPulled.delete(name)
+        this.emit('pullEnd', {repoName: name})
     }
 
     path(name) {
@@ -87,16 +111,11 @@ class Clones {
         return stdout.trim()
     }
 
-    // `onFetchStart` is called just after
-    // `clones.isBeingFetched(name)` becomes `true`
-    async clone(url, name, options = {}) {
-        const onFetchStart = options.onFetchStart || (() => {})
-
-        assert(!this.isBeingFetched(name))
+    async clone(url, name, options) {
+        assert(!options, 'deprecated')
 
         try {
-            this._beingFetched.add(name)
-            onFetchStart()
+            this._emitPullStart(name)
 
             await this.remove(name)
 
@@ -116,18 +135,17 @@ class Clones {
                 {timeout: 60 * 1000},
             )
         } finally {
-            this._beingFetched.delete(name)
+            this._emitPullEnd(name)
         }
     }
 
-    async pull(name, options = {}) {
-        const onFetchStart = options.onFetchStart || (() => {})
+    async pull(name, options) {
+        assert(!options, 'deprecated')
 
         const dir = this.path(name)
 
         try {
-            this._beingFetched.add(name)
-            onFetchStart()
+            this._emitPullStart(name)
 
             const branch = await this._getCurrentBranch(name)
 
@@ -155,7 +173,7 @@ class Clones {
                 ]
             )
         } finally {
-            this._beingFetched.delete(name)
+            this._emitPullEnd(name)
         }
     }
 

server/src/clones.test.js

@@ -29,7 +29,7 @@ describe('clones', () => {
         await clones.remove('r')
         assert(!await clones.exists('r'))
         assert(!await fileExists(path.join(basePath, 'r', 'package.json')))
-    }).timeout(10 * 1000)
+    }).timeout(20 * 1000)
 
     it('fails with a nonexistent repo', async () => {
         try {
@@ -41,5 +41,5 @@ describe('clones', () => {
             return
         }
         assert(false)
-    }).timeout(10 * 1000)
+    }).timeout(20 * 1000)
 })

server/src/create-test-server.js

@@ -3,6 +3,7 @@ const createSearches = require('./searches')
 const {dummyLogger} = require('./log')
 const createDb = require('./db')
 const createClones = require('./clones')
+const createPullCron = require('./pull-cron')
 const serve = require('./serve')
 
 module.exports = async () => {
@@ -16,9 +17,10 @@ module.exports = async () => {
     const clones = createClones('test-clones')
     const searches = createSearches()
     const log = dummyLogger
+    const pullCron = createPullCron({db, clones, log})
 
     const server = await serve({
-        db, clones, searches, log,
+        db, clones, searches, log, pullCron,
         address, port,
     })
 

server/src/db.js

@@ -6,7 +6,7 @@ const {promisify} = require('util')
 
 const migrate = require('./migrate')
 const {randomString} = require('./util')
-const sqlite = require('./sqlite-promise')
+const createSqliteDriver = require('./sqlite-promise')
 
 const V = require('@motet_a/validate')
 
@@ -17,14 +17,19 @@ const repoSpec = V.shape({
     webUrl: V.string.max(256),
     gitUrl: V.string.min(1).max(256),
     cloned: V.boolean,
-    fetchFailed: V.boolean,
+    pullFailed: V.boolean,
 })
 
-const repoFromSQLite = dbRepo =>
-    dbRepo && Object.assign({}, dbRepo, {
+const repoFromSQLite = dbRepo => {
+    if (!dbRepo) {
+        return
+    }
+
+    return Object.assign({}, dbRepo, {
         cloned: !!dbRepo.cloned,
-        fetchFailed: !!dbRepo.fetchFailed,
+        pullFailed: !!dbRepo.pullFailed,
     })
+}
 
 const errors = {
     ALREADY_EXISTS: 'ALREADY_EXISTS',
@@ -43,6 +48,9 @@ const newDbError = code => {
 const isDbError = error =>
     error && error[dbErrorSymbol] === dbErrorSymbol
 
+const nowTimestamp = () =>
+    new Date().getTime()
+
 class Db {
     constructor(db) {
         assert(db)
@@ -63,6 +71,20 @@ class Db {
         return repos.map(repoFromSQLite)
     }
 
+    async getRepoNamesToPull(defaultPullDelay) {
+        const repos = await this._db.all(
+            `SELECT name, pulledAt, cloned, pullDelay
+            FROM repos
+            WHERE cloned AND (
+                coalesce(pulledAt, 0) + coalesce(pullDelay, ?)
+            ) < ?;`,
+            defaultPullDelay,
+            nowTimestamp()
+        )
+
+        return repos.map(r => r.name)
+    }
+
     async getRepo(name) {
         const repo = await this._db.get(
             `SELECT *
@@ -80,13 +102,13 @@ class Db {
         try {
             return await this._db.run(
                 `INSERT INTO repos (
-                    name, webUrl, gitUrl, cloned, fetchFailed
+                    name, webUrl, gitUrl, cloned, pullFailed
                 ) VALUES (?, ?, ?, ?, ?);`,
                 repo.name,
                 repo.webUrl,
                 repo.gitUrl,
                 repo.cloned,
-                repo.fetchFailed
+                repo.pullFailed
             )
         } catch (error) {
             if (error.code === 'SQLITE_CONSTRAINT' &&
@@ -99,12 +121,13 @@ class Db {
     }
 
     // Returns a Promise
-    setRepoFetchFailed(name, fetchFailed) {
+    setRepoPullFailed(name, pullFailed) {
         return this._db.run(
             `UPDATE repos
-            SET fetchFailed = ?
+            SET pullFailed = ?, pulledAt = ?
             WHERE name = ?;`,
-            fetchFailed,
+            pullFailed,
+            nowTimestamp(),
             name
         )
     }
@@ -113,9 +136,10 @@ class Db {
     setRepoCloned(name, cloned) {
         return this._db.run(
             `UPDATE repos
-            SET cloned = ?
+            SET cloned = ?, pulledAt = ?
             WHERE name = ?;`,
             cloned,
+            nowTimestamp(),
             name
         )
     }
@@ -206,12 +230,12 @@ const createDb = async (fileName, options = {}) => {
         __dirname, '..', 'var', fileName + '.sqlitedb'
     )
 
-    const rawDB = await sqlite(filePath)
+    const sqliteDriver = await createSqliteDriver(filePath)
     if (dropTables) {
-        await rawDB.dropTables()
+        await sqliteDriver.dropTables()
     }
-    await migrate(rawDB)
-    const db = new Db(rawDB)
+    await migrate(sqliteDriver)
+    const db = new Db(sqliteDriver)
     return db
 }