diff --git a/src/core/Repository.ts b/src/core/Repository.ts index e99dc58..5e09922 100644 --- a/src/core/Repository.ts +++ b/src/core/Repository.ts @@ -552,8 +552,8 @@ export default class Repository { async isReady() { if (this.status !== RepositoryStatus.READY) return false; if (!this.hasEmptyTree() && !(await FileModel.exists({ repoId: this.repoId, treeGeneration: this.model.treeGeneration || { $exists: false } }).exec())) { - this.model.status = RepositoryStatus.PREPARING; - await this.updateIfNeeded({ force: true }); + // Missing-tree recovery competes with owner refreshes for the same job. + await this.refresh(); return false; } return true; @@ -709,7 +709,8 @@ export default class Repository { } if ( this.model.source.commit == newCommit && - this.status == RepositoryStatus.READY + this.status == RepositoryStatus.READY && + !opt?.force ) { logger.info("up to date", { repoId: this._model.repoId }); return; diff --git a/test/performance-integration.test.js b/test/performance-integration.test.js index 994079b..c0bc500 100644 --- a/test/performance-integration.test.js +++ b/test/performance-integration.test.js @@ -389,7 +389,7 @@ const uri = process.env.PERFORMANCE_MONGO_URI; expect(response).to.deep.equal({ "": 1, src: 1 }); await File.deleteMany({ repoId: model.repoId, treeGeneration: "active" }); let refreshed = false; - repo.updateIfNeeded = async ({ force }) => { refreshed = force; }; + repo.refresh = async () => { refreshed = true; }; expect(await repo.isReady()).to.equal(false); expect(refreshed).to.equal(true); } finally { utils.getRepo = getRepo; } }); @@ -716,7 +716,7 @@ const uri = process.env.PERFORMANCE_MONGO_URI; expect((await Repo.findById(model._id)).emptyTreeGeneration).to.equal(undefined); await File.deleteMany({ repoId: model.repoId }); let refreshed = false; - fresh.updateIfNeeded = async () => { refreshed = true; }; + fresh.refresh = async () => { refreshed = true; }; expect(await fresh.isReady()).to.equal(false); expect(refreshed).to.equal(true); }); diff --git a/test/repository-refresh.test.js b/test/repository-refresh.test.js index fd1dd25..0dd2be0 100644 --- a/test/repository-refresh.test.js +++ b/test/repository-refresh.test.js @@ -129,6 +129,46 @@ describe("repository refresh and restoration", () => { return stored; } + for (const force of [false, true]) { + for (const commit of ["saved-sha", "new-sha"]) { + it(`refreshes a ready snapshot with force=${force}, commit=${commit}`, async () => { + const repo = repository("ready"); + repo.model.options.update = true; + repo.model.lastView = new Date(0); + const stored = database(repo); + repo.getToken = async () => "token"; + stub(github, "getRepositoryFromGitHub", async options => { + expect(options.force).to.equal(true); + return { fullName: "owner/repo", model: { defaultBranch: "main" }, + branches: async options => { + expect(options.force).to.equal(true); + return [{ name: "main", commit }]; + }, getCommitInfo: async () => ({ commit: {} }) }; + }); + let cleared = false, queued = false; + repo.resetSate = async () => { + expect(stored.source.commit).to.equal(commit); + expect(stored.status).to.equal("preparing"); + cleared = true; + }; + stub(queue, "downloadQueue", { getJob: async () => undefined, add: async (_id, data) => { + expect(cleared).to.equal(true); + expect(data.repoId).to.equal("restore-me"); + queued = true; + } }); + if (force) await repo.refresh(); + else await repo.updateIfNeeded(); + const rebuilt = force || commit !== "saved-sha"; + expect(cleared).to.equal(rebuilt); + expect(queued).to.equal(rebuilt); + expect(stored.status).to.equal(rebuilt ? "preparing" : "ready"); + expect(stored.source.commit).to.equal(commit); + expect(stored.repoId).to.equal("restore-me"); + expect(stored).not.to.have.property("refreshToken"); + }); + } + } + it("claims a lease before GitHub work and rejects a concurrent refresh", async () => { const first = repository("ready"); const second = new Repository(new Model(first.model.toObject())); @@ -151,6 +191,35 @@ describe("repository refresh and restoration", () => { expect(stored).not.to.have.property("refreshToken"); }); + for (const recoveryFirst of [false, true]) { + it(`serializes missing-tree recovery and dashboard refresh (recovery first: ${recoveryFirst})`, async () => { + const first = repository("ready"); + const second = new Repository(new Model(first.model.toObject())); + const stored = database(first); + const Files = require("../src/core/model/files/files.model").default; + stub(Files, "exists", () => ({ exec: async () => null })); + let release, started; + const entered = new Promise(resolve => { started = resolve; }); + let updates = 0; + first.updateIfNeeded = async () => { + updates++; + started(); + await new Promise(resolve => { release = resolve; }); + }; + second.updateIfNeeded = async () => { updates++; }; + const running = recoveryFirst ? first.isReady() : first.refresh(); + await entered; + try { + let failure; + try { await (recoveryFirst ? second.refresh() : second.isReady()); } catch (error) { failure = error; } + expect(failure?.message).to.equal("invalid_status"); + expect(updates).to.equal(1); + expect(stored.status).to.equal("ready"); + } finally { release(); await running; } + expect(stored).not.to.have.property("refreshToken"); + }); + } + it("releases a failed lease so reconnecting can be retried", async () => { const repo = repository("removed"); const stored = database(repo);