diff --git a/libs/datatug/main/src/lib/project/new-project/new-project-form.component.spec.ts b/libs/datatug/main/src/lib/project/new-project/new-project-form.component.spec.ts index abbb963d..7a442922 100644 --- a/libs/datatug/main/src/lib/project/new-project/new-project-form.component.spec.ts +++ b/libs/datatug/main/src/lib/project/new-project/new-project-form.component.spec.ts @@ -2,11 +2,12 @@ import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; import { ComponentFixture, TestBed } from '@angular/core/testing'; import { PopoverController } from '@ionic/angular'; import { ErrorLogger } from '@sneat/core'; -import { Subject, of } from 'rxjs'; +import { Subject, of, throwError } from 'rxjs'; import { NewProjectFormComponent } from './new-project-form.component'; import { DatatugNavService } from '../../services/nav/datatug-nav.service'; import { ProjectService } from '../../services/project/project.service'; +import { GithubRepoError } from '../../services/repo/github/github-api'; import { GithubProjectCreateService } from '../../services/repo/github/github-project-create.service'; import { GithubOAuthService } from '../../services/repo/github/github-oauth.service'; import { GithubReposService } from '../../services/repo/github/github-repos.service'; @@ -337,6 +338,50 @@ describe('NewProjectFormComponent creating in a GitHub repo', () => { ); }); + it('shows what GitHub did not return when it made the repository but sent no default branch', () => { + oauth.isSignedIn = true; + oauth.accessToken = 'gho_token'; + (component as unknown as { selectedRepo: { set: (v: string) => void } }).selectedRepo.set( + '__new__', + ); + component.newRepoName = 'my-projects'; + repos.createRepo = vi.fn(() => + throwError( + () => + new GithubRepoError( + 'default-branch', + 'GitHub created my-projects, but did not return the default branch of my-projects, so DataTug cannot tell which branch to commit to.', + ), + ), + ); + component.store = 'github'; + + component.create(); + + expect(formErrorOf(component)).toContain('did not return the default branch'); + expect(createProject).not.toHaveBeenCalled(); + }); + + it('shows what GitHub did not return when the chosen repository has no default branch', () => { + oauth.isSignedIn = true; + oauth.accessToken = 'gho_token'; + (component as unknown as { selectedRepo: { set: (v: string) => void } }).selectedRepo.set( + 'datatug/demo-projects', + ); + component.store = 'github'; + component.title = 'My project'; + + component.create(); + createProject$.error( + new GithubRepoError( + 'default-branch', + 'GitHub did not return the default branch of datatug/demo-projects, so DataTug cannot tell which branch to commit to.', + ), + ); + + expect(formErrorOf(component)).toContain('did not return the default branch'); + }); + it('requires a name before creating a new repository', () => { oauth.isSignedIn = true; oauth.accessToken = 'gho_token'; diff --git a/libs/datatug/main/src/lib/project/new-project/new-project-form.component.ts b/libs/datatug/main/src/lib/project/new-project/new-project-form.component.ts index a8bd1431..4fe704b9 100644 --- a/libs/datatug/main/src/lib/project/new-project/new-project-form.component.ts +++ b/libs/datatug/main/src/lib/project/new-project/new-project-form.component.ts @@ -25,7 +25,10 @@ import { IProjectContext, parseDatatugStoreRef } from '../../nav/nav-models'; import { DatatugNavService } from '../../services/nav/datatug-nav.service'; import { DatatugServicesProjectModule } from '../../services/project/datatug-services-project.module'; import { ProjectService } from '../../services/project/project.service'; -import { IGithubRepo } from '../../services/repo/github/github-api'; +import { + GithubRepoError, + IGithubRepo, +} from '../../services/repo/github/github-api'; import { GithubOAuthService, GithubSignInRedirecting, @@ -249,7 +252,9 @@ export class NewProjectFormComponent implements ViewDidEnter { error: (err) => { this.isCreating.set(false); this.formError.set( - `Failed to create the repository "${name}" on GitHub.`, + err instanceof GithubRepoError + ? err.message + : `Failed to create the repository "${name}" on GitHub.`, ); this.errorLogger.logError(err, 'Failed to create a GitHub repo'); }, @@ -283,7 +288,9 @@ export class NewProjectFormComponent implements ViewDidEnter { error: (err) => { this.isCreating.set(false); this.formError.set( - `Failed to create the project in ${fullName}. Check that your GitHub access allows writing to it.`, + err instanceof GithubRepoError + ? err.message + : `Failed to create the project in ${fullName}. Check that your GitHub access allows writing to it.`, ); this.errorLogger.logError( err, diff --git a/libs/datatug/main/src/lib/services/repo/datatug-store.service.github.spec.ts b/libs/datatug/main/src/lib/services/repo/datatug-store.service.github.spec.ts index b0c397bf..c3933a8c 100644 --- a/libs/datatug/main/src/lib/services/repo/datatug-store.service.github.spec.ts +++ b/libs/datatug/main/src/lib/services/repo/datatug-store.service.github.spec.ts @@ -52,7 +52,7 @@ describe('DatatugStoreGithubService.watchProjectItem', () => { function createService(getRawJson: ReturnType) { TestBed.configureTestingModule({ providers: [ - { provide: GithubProjectReaderService, useValue: { getRawJson } }, + { provide: GithubProjectReaderService, useValue: { getRawJson, visitEpoch: () => '0.0' } }, ], }); return TestBed.inject(DatatugStoreGithubService); @@ -113,7 +113,7 @@ describe('DatatugStoreGithubService.getProjectSummary: ids it cannot read', () = function createService(getRawJson: ReturnType) { TestBed.configureTestingModule({ providers: [ - { provide: GithubProjectReaderService, useValue: { getRawJson } }, + { provide: GithubProjectReaderService, useValue: { getRawJson, visitEpoch: () => '0.0' } }, ], }); return TestBed.inject(DatatugStoreGithubService); @@ -143,7 +143,11 @@ describe('DatatugStoreGithubService.getProjectSummary: reads through the reader providers: [ { provide: GithubProjectReaderService, - useValue: { getRawJson, readInfo: () => of({ state }) }, + useValue: { + getRawJson, + visitEpoch: () => '0.0', + readInfo: () => of({ state }), + }, }, ], }); @@ -170,3 +174,41 @@ describe('DatatugStoreGithubService.getProjectSummary: reads through the reader expect((error as GithubProjectNotFoundError).reason).toBe(state); }); }); + +describe('DatatugStoreGithubService.getProjectSummary: the summary follows the commit of the reader', () => { + it('a summary read before the reader moved to another commit is read again, once; until it moves again it is kept', () => { + let epoch = '0.0'; + let title = 'at the old commit'; + const getRawJson = vi.fn(() => of({ id: 'r@o@d', title })); + TestBed.configureTestingModule({ + providers: [ + { + provide: GithubProjectReaderService, + useValue: { getRawJson, visitEpoch: () => epoch }, + }, + ], + }); + const service = TestBed.inject(DatatugStoreGithubService); + const titles: string[] = []; + const read = () => + service + .getProjectSummary('r@o@d') + .subscribe((p) => titles.push(p.title as string)); + + read(); + read(); + expect(getRawJson).toHaveBeenCalledTimes(1); + + epoch = '0.1'; // the visit moved to another commit + title = 'at the new commit'; + read(); + read(); + expect(getRawJson).toHaveBeenCalledTimes(2); + expect(titles).toEqual([ + 'at the old commit', + 'at the old commit', + 'at the new commit', + 'at the new commit', + ]); + }); +}); diff --git a/libs/datatug/main/src/lib/services/repo/datatug-store.service.github.ts b/libs/datatug/main/src/lib/services/repo/datatug-store.service.github.ts index 28e2935e..1228d858 100644 --- a/libs/datatug/main/src/lib/services/repo/datatug-store.service.github.ts +++ b/libs/datatug/main/src/lib/services/repo/datatug-store.service.github.ts @@ -1,7 +1,7 @@ import { IFolder, IFolderItem } from '../../models/definition/folder'; import { IProjectSummary } from '../../models/definition/project'; import { IDatatugStoreService } from './datatug-store.service.interface'; -import { Observable, defer, of, throwError } from 'rxjs'; +import { Observable, of, throwError } from 'rxjs'; import { map, shareReplay, switchMap } from 'rxjs/operators'; import { Injectable, inject } from '@angular/core'; import { @@ -34,9 +34,11 @@ export class DatatugStoreGithubService implements IDatatugStoreService { // reads the project summary (for its `boards` list), so without this a // page that renders both the project summary AND the folder tabs (the // project page itself) would fetch `datatug-project.json` twice. + // Kept with the reader's `visitEpoch` at the time: a summary read at a commit the visit has since moved away from + // (the reader asked GitHub again about a commit it only remembered, and was told another) is not served again. private readonly summaryCache = new Map< string, - Observable + { readonly epoch: string; readonly summary: Observable } >(); /** @@ -55,58 +57,57 @@ export class DatatugStoreGithubService implements IDatatugStoreService { } getProjectSummary(projectId: string): Observable { - let cached = this.summaryCache.get(projectId); - if (cached) { - return cached; + const cached = this.summaryCache.get(projectId); + if (cached && cached.epoch === this.githubReader.visitEpoch(projectId)) { + return cached.summary; } try { assertReadableGithubProjectId(projectId); } catch (err) { return throwError(() => err); } + const epoch = this.githubReader.visitEpoch(projectId); // Through the reader: the same commit as the listing and every other file, one cache, one request (the old // second, separate read of this file could come from a different commit than the rest of the page). - // `defer`: `shareReplay` drops a failed read and the next subscriber reads again, instead of getting the same + // The reader's observable is lazy (a read starts on subscription, a failed one is read again by the next + // subscriber), and `shareReplay` drops a failed read: the next subscriber reads again, instead of getting the same // failure back until the page is reloaded. - cached = defer(() => - this.githubReader.getRawJson( - projectId, - 'datatug-project.json', - ), - ).pipe( - switchMap((p) => - p - ? of(p) - : this.githubReader - .readInfo(projectId) - .pipe( - switchMap((info) => - throwError( - () => - new GithubProjectNotFoundError( - projectId, - info.state === 'moved' ? 'moved' : 'missing', - ), + const summary = this.githubReader + .getRawJson(projectId, 'datatug-project.json') + .pipe( + switchMap((p) => + p + ? of(p) + : this.githubReader + .readInfo(projectId) + .pipe( + switchMap((info) => + throwError( + () => + new GithubProjectNotFoundError( + projectId, + info.state === 'moved' ? 'moved' : 'missing', + ), + ), ), ), - ), - ), - map((p) => { - if (p.id === projectId) { - return p; - } - if (p.id) { - console.warn( - `Request project info with projectId=${projectId} but response JSON have id=${p.id}`, - ); - } - return { ...p, id: projectId }; - }), - shareReplay(1), - ); - this.summaryCache.set(projectId, cached); - return cached; + ), + map((p) => { + if (p.id === projectId) { + return p; + } + if (p.id) { + console.warn( + `Request project info with projectId=${projectId} but response JSON have id=${p.id}`, + ); + } + return { ...p, id: projectId }; + }), + shareReplay(1), + ); + this.summaryCache.set(projectId, { epoch, summary }); + return summary; } /** diff --git a/libs/datatug/main/src/lib/services/repo/github/github-api.spec.ts b/libs/datatug/main/src/lib/services/repo/github/github-api.spec.ts new file mode 100644 index 00000000..d9bb81a3 --- /dev/null +++ b/libs/datatug/main/src/lib/services/repo/github/github-api.spec.ts @@ -0,0 +1,110 @@ +import { describe, expect, it } from 'vitest'; + +import { GithubRepoError, requireGithubRepo, toGithubRepo } from './github-api'; + +describe('toGithubRepo', () => { + it('maps a complete payload', () => { + expect( + toGithubRepo({ + full_name: 'datatug/demo', + private: true, + default_branch: 'trunk', + }), + ).toEqual({ + fullName: 'datatug/demo', + private: true, + defaultBranch: 'trunk', + }); + }); + + it('builds the full name from the owner and the name, and reads a missing private as public', () => { + expect( + toGithubRepo({ + owner: { login: 'datatug' }, + name: 'demo', + default_branch: 'main', + }), + ).toEqual({ + fullName: 'datatug/demo', + private: false, + defaultBranch: 'main', + }); + }); + + it.each([ + ['no full name', { default_branch: 'main' }], + [ + 'an owner without a name', + { owner: { login: 'datatug' }, default_branch: 'main' }, + ], + [ + 'no default branch (never assumed to be main)', + { full_name: 'datatug/demo' }, + ], + [ + 'an empty default branch', + { full_name: 'datatug/demo', default_branch: '' }, + ], + ])('is incomplete with %s', (_name, wire) => { + expect(toGithubRepo(wire)).toBeUndefined(); + }); +}); + +describe('requireGithubRepo', () => { + it('returns the repository when it is complete', () => { + expect( + requireGithubRepo( + { full_name: 'datatug/demo', default_branch: 'main' }, + 'x', + ).defaultBranch, + ).toBe('main'); + }); + + it.each([ + [ + 'no default branch', + { full_name: 'datatug/demo' }, + false, + 'default-branch', + 'GitHub did not return the default branch of datatug/demo, so DataTug cannot tell which branch to commit to.', + ], + [ + 'no default branch, owner and name given', + { owner: { login: 'datatug' }, name: 'demo' }, + false, + 'default-branch', + 'GitHub did not return the default branch of datatug/demo, so DataTug cannot tell which branch to commit to.', + ], + [ + 'no default branch of a repository just created', + { full_name: 'datatug/demo' }, + true, + 'default-branch', + 'GitHub created datatug/demo, but did not return the default branch of datatug/demo, so DataTug cannot tell which branch to commit to.', + ], + [ + 'no name', + {}, + false, + 'full-name', + 'GitHub did not return the repository datatug/demo', + ], + [ + 'no name of a repository just created', + { default_branch: 'main' }, + true, + 'full-name', + 'GitHub created datatug/demo, but did not return its full name', + ], + ])('says what is missing: %s', (_name, wire, created, missing, message) => { + let error: unknown; + try { + requireGithubRepo(wire, 'datatug/demo', created); + } catch (err) { + error = err; + } + expect(error).toBeInstanceOf(GithubRepoError); + expect((error as GithubRepoError).missing).toBe(missing); + expect((error as GithubRepoError).message).toBe(message); + }); +}); diff --git a/libs/datatug/main/src/lib/services/repo/github/github-api.ts b/libs/datatug/main/src/lib/services/repo/github/github-api.ts index 9d01a294..2a522953 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-api.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-api.ts @@ -32,7 +32,25 @@ export interface IGithubRepoWire { readonly default_branch?: string; } -/** Maps a GitHub repository payload to {@link IGithubRepo}, or undefined when incomplete. */ +/** + * A repository payload that cannot be used, and what is missing from it. The message is shown to the user (the new + * project form), so it says what GitHub did not return rather than guessing: in particular there is no default + * branch to assume, since a project committed to the wrong branch would not be the one the reader reads (`HEAD`). + */ +export class GithubRepoError extends Error { + constructor( + public readonly missing: 'full-name' | 'default-branch', + message: string, + ) { + super(message); + this.name = 'GithubRepoError'; + } +} + +/** + * Maps a GitHub repository payload to {@link IGithubRepo}, or undefined when incomplete: no full name, or no + * default branch (never assumed to be `main`, see {@link GithubRepoError}). + */ export function toGithubRepo( wire: IGithubRepoWire, ): IGithubRepo | undefined { @@ -41,12 +59,42 @@ export function toGithubRepo( (wire.owner?.login && wire.name ? `${wire.owner.login}/${wire.name}` : undefined); - if (!fullName) { + if (!fullName || !wire.default_branch) { return undefined; } return { fullName, private: !!wire.private, - defaultBranch: wire.default_branch || 'main', + defaultBranch: wire.default_branch, }; } + +/** + * The same, for a repository the caller needs in full: throws a {@link GithubRepoError} naming what GitHub did not + * return. `name` is what the caller calls the repository (`owner/repo`, or the name given for a new one); + * `created` says the repository has just been made, so the message does not let the user make it twice. + */ +export function requireGithubRepo( + wire: IGithubRepoWire, + name: string, + created = false, +): IGithubRepo { + const repo = toGithubRepo(wire); + if (repo) { + return repo; + } + const hasName = !!wire.full_name || (!!wire.owner?.login && !!wire.name); + const lead = created ? `GitHub created ${name}, but` : 'GitHub'; + if (hasName) { + throw new GithubRepoError( + 'default-branch', + `${lead} did not return the default branch of ${name}, so DataTug cannot tell which branch to commit to.`, + ); + } + throw new GithubRepoError( + 'full-name', + created + ? `${lead} did not return its full name` + : `GitHub did not return the repository ${name}`, + ); +} diff --git a/libs/datatug/main/src/lib/services/repo/github/github-fake-backend.test.ts b/libs/datatug/main/src/lib/services/repo/github/github-fake-backend.test.ts index 6f8e1fe4..0adea9b1 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-fake-backend.test.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-fake-backend.test.ts @@ -235,14 +235,17 @@ export class FakeGithub { */ export class ManualTimers { private nextId = 1; - private readonly pending = new Map void>(); + private readonly pending = new Map< + number, + { fire: () => void; ms: number } + >(); /** The delay of every timer ever set, in order. */ readonly delays: number[] = []; readonly set: GithubSetTimer = (fire, ms) => { const id = this.nextId++; this.delays.push(ms); - this.pending.set(id, fire); + this.pending.set(id, { fire, ms }); return () => { this.pending.delete(id); }; @@ -256,11 +259,21 @@ export class ManualTimers { fireAll(): void { const fires = [...this.pending.values()]; this.pending.clear(); - for (const fire of fires) { + for (const { fire } of fires) { fire(); } } + /** Fires, and forgets, only the waiting timers that were set for `ms`. */ + fireWith(ms: number): void { + for (const [id, timer] of [...this.pending]) { + if (timer.ms === ms) { + this.pending.delete(id); + timer.fire(); + } + } + } + /** Resolves once at least `n` timers are waiting (the code under test has reached its first await). */ async waitFor(n = 1): Promise { for (let i = 0; i < 1000 && this.pending.size < n; i++) { diff --git a/libs/datatug/main/src/lib/services/repo/github/github-file-store-api.spec.ts b/libs/datatug/main/src/lib/services/repo/github/github-file-store-api.spec.ts index 1310ed99..db9a51ac 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-file-store-api.spec.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-file-store-api.spec.ts @@ -2,6 +2,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest'; import { ManualTimers } from './github-fake-backend.test'; import { + GITHUB_STORE_OPEN_TIMEOUT_MS, GITHUB_STORE_TIMEOUT_MS, guardGithubFileStore, setGithubTimer, @@ -22,6 +23,7 @@ function spyStore(answer: () => Promise) { putFile: call('putFile'), getResolved: call('getResolved'), putResolved: call('putResolved'), + dropResolved: call('dropResolved'), forgetResolved: call('forgetResolved'), } as unknown as IGithubFileStore; return { store, calls }; @@ -38,6 +40,7 @@ describe('guardGithubFileStore (a cache that does not answer is no cache)', () = putFile: () => Promise.resolve(), getResolved: () => Promise.resolve({ sha: 's', at: 1 }), putResolved: () => Promise.resolve(), + dropResolved: () => Promise.resolve(), forgetResolved: () => Promise.resolve(), }, timers.set, @@ -46,12 +49,18 @@ describe('guardGithubFileStore (a cache that does not answer is no cache)', () = expect(await store.getResolved('o/r@HEAD')).toEqual({ sha: 's', at: 1 }); await store.putFile(KEY, 'f', { text: 'x', bytes: 1 }); await store.putResolved('o/r@HEAD', { sha: 's', at: 1 }); + await store.dropResolved('o/r@HEAD'); await store.forgetResolved('o/r'); - expect(timers.delays).toEqual(Array(5).fill(GITHUB_STORE_TIMEOUT_MS)); + // Every call waits the same; the first also starts the limit of the opening, cancelled when it answers. + expect(timers.delays).toEqual([ + GITHUB_STORE_TIMEOUT_MS, + GITHUB_STORE_OPEN_TIMEOUT_MS, + ...Array(5).fill(GITHUB_STORE_TIMEOUT_MS), + ]); expect(timers.count).toBe(0); }); - it('a call that does not answer in time gives the empty answer, and the store is off for the rest of the visit', async () => { + it('a first call that does not answer in time gives the empty answer; the calls after it are not made until the store answers', async () => { const timers = new ManualTimers(); const { store: inner, calls } = spyStore( () => new Promise(() => undefined), @@ -60,19 +69,156 @@ describe('guardGithubFileStore (a cache that does not answer is no cache)', () = const first = store.getResolved('o/r@HEAD'); const second = store.getFile(KEY, 'f'); // asked at the same time - expect(timers.count).toBe(2); - timers.fireAll(); + expect(timers.count).toBe(3); // two waits, and the one limit of the opening + timers.fireWith(GITHUB_STORE_TIMEOUT_MS); expect(await first).toBeUndefined(); expect(await second).toBeUndefined(); - // Off: not asked, no timer. + // Paused: not asked, no timer, no wait. expect(await store.getFile(KEY, 'g')).toBeUndefined(); await store.putFile(KEY, 'g', { text: 'x', bytes: 1 }); await store.putResolved('o/r@HEAD', { sha: 's', at: 1 }); - await store.forgetResolved('o/r'); expect(await store.getResolved('o/r@HEAD')).toBeUndefined(); expect(calls).toEqual(['getResolved', 'getFile']); - expect(timers.count).toBe(0); + expect(timers.count).toBe(1); // the limit of the opening + }); + + it('a late answer of the first call switches the cache back on for the calls after it, and is not applied to the read that gave up', async () => { + const timers = new ManualTimers(); + const answers: ((v: unknown) => void)[] = []; + let slow = true; + const { store: inner, calls } = spyStore(() => + slow + ? new Promise((resolve) => answers.push(resolve)) + : Promise.resolve({ text: 'cached', bytes: 6 }), + ); + const store = guardGithubFileStore(inner, timers.set); + + const read = store.getFile(KEY, 'f'); + timers.fireWith(GITHUB_STORE_TIMEOUT_MS); + expect(await read).toBeUndefined(); + expect(await store.getFile(KEY, 'g')).toBeUndefined(); // paused: not asked + + slow = false; + answers[0]({ text: 'late', bytes: 4 }); // the database opened, a moment too late for the first read + await Promise.resolve(); + await Promise.resolve(); + expect(await read).toBeUndefined(); + expect(timers.count).toBe(0); // the limit of the opening was cancelled by the answer + + expect(await store.getFile(KEY, 'h')).toEqual({ text: 'cached', bytes: 6 }); + expect(calls).toEqual(['getFile', 'getFile']); + }); + + it('the wait of a call that began before the cache was heard from does not turn off a cache that has since been proven', async () => { + const timers = new ManualTimers(); + // The timers of the waits of the calls, one each, in the order the calls were made. + const waits: (() => void)[] = []; + const setTimer: typeof timers.set = (fire, ms) => { + if (ms === GITHUB_STORE_TIMEOUT_MS) { + waits.push(fire); + } + return timers.set(fire, ms); + }; + const answers: ((v: unknown) => void)[] = []; + let instant = false; + const { store: inner, calls } = spyStore(() => + instant + ? Promise.resolve({ text: 'cached', bytes: 6 }) + : new Promise((resolve) => answers.push(resolve)), + ); + const store = guardGithubFileStore(inner, setTimer); + + const first = store.getFile(KEY, 'f'); // page 1: the database is still opening + const second = store.getFile(KEY, 'g'); // page 2, a moment later: not heard from yet either + waits[0](); // the first wait ends: paused + expect(await first).toBeUndefined(); + + instant = true; + answers[0]({ text: 'late', bytes: 4 }); // the database opens: the cache works + await Promise.resolve(); + await Promise.resolve(); + expect(await store.getFile(KEY, 'h')).toEqual({ text: 'cached', bytes: 6 }); + + waits[1](); // the second call's wait ends, though it began before the cache was proven + expect(await second).toBeUndefined(); + const before = calls.length; + expect(await store.getFile(KEY, 'i')).toEqual({ text: 'cached', bytes: 6 }); + expect(calls.length).toBe(before + 1); // the cache is still asked + }); + + it('a first call that does not answer within the limit of the opening: the cache is off for the rest of the visit, and a later answer changes nothing', async () => { + const timers = new ManualTimers(); + let answer: (v: unknown) => void = () => undefined; + const { store: inner, calls } = spyStore( + () => new Promise((resolve) => (answer = resolve)), + ); + const store = guardGithubFileStore(inner, timers.set); + + const read = store.getResolved('o/r@HEAD'); + timers.fireWith(GITHUB_STORE_TIMEOUT_MS); + await read; + timers.fireWith(GITHUB_STORE_OPEN_TIMEOUT_MS); + answer({ sha: 's', at: 1 }); + await Promise.resolve(); + await Promise.resolve(); + + expect(await store.getResolved('o/r@HEAD')).toBeUndefined(); + expect(calls).toEqual(['getResolved']); + }); + + it('the limit of the opening is longer than the wait of a read', () => { + expect(GITHUB_STORE_OPEN_TIMEOUT_MS).toBeGreaterThan( + GITHUB_STORE_TIMEOUT_MS, + ); + expect(GITHUB_STORE_OPEN_TIMEOUT_MS).toBeLessThanOrEqual(5000); + }); + + it('a store that has answered and then does not answer in time is off for the rest of the visit, and a late answer does not turn it on', async () => { + const timers = new ManualTimers(); + let hang = false; + let answer: (v: unknown) => void = () => undefined; + const { store: inner, calls } = spyStore(() => + hang + ? new Promise((resolve) => (answer = resolve)) + : Promise.resolve(undefined), + ); + const store = guardGithubFileStore(inner, timers.set); + await store.getFile(KEY, 'works'); + + hang = true; + const read = store.getFile(KEY, 'f'); + timers.fireAll(); + expect(await read).toBeUndefined(); + answer({ text: 'late', bytes: 4 }); + await Promise.resolve(); + await Promise.resolve(); + + expect(await store.getFile(KEY, 'g')).toBeUndefined(); + expect(await store.getResolved('o/r@HEAD')).toBeUndefined(); + expect(calls).toEqual(['getFile', 'getFile']); + }); + + it('the deletion of remembered answers is attempted whatever the state of the cache, with the same wait', async () => { + const timers = new ManualTimers(); + const { store: inner, calls } = spyStore( + () => new Promise(() => undefined), + ); + const store = guardGithubFileStore(inner, timers.set); + const read = store.getFile(KEY, 'f'); + timers.fireAll(); // paused, then given up on + await read; + + const dropped = store.dropResolved('o/r@HEAD'); + const forgotten = store.forgetResolved('o/r'); + expect(calls).toEqual(['getFile', 'dropResolved', 'forgetResolved']); + expect(timers.count).toBe(2); + timers.fireAll(); + expect(await dropped).toBeUndefined(); + expect(await forgotten).toBeUndefined(); + // The others are still not made. + expect(await store.getFile(KEY, 'g')).toBeUndefined(); + expect(calls).toHaveLength(3); }); it('a call that fails, even by throwing, is an empty answer; the store stays on', async () => { @@ -92,6 +238,7 @@ describe('guardGithubFileStore (a cache that does not answer is no cache)', () = putFile: () => Promise.resolve(), getResolved: () => Promise.resolve(undefined), putResolved: () => Promise.resolve(), + dropResolved: () => Promise.resolve(), forgetResolved: () => Promise.resolve(), }, timers.set, diff --git a/libs/datatug/main/src/lib/services/repo/github/github-file-store-api.ts b/libs/datatug/main/src/lib/services/repo/github/github-file-store-api.ts index 5f460206..4e3b8f28 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-file-store-api.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-file-store-api.ts @@ -33,6 +33,12 @@ export interface IGithubFileStore { /** The commit remembered for `/@` (`HEAD` for the default branch). */ getResolved(key: string): Promise; putResolved(key: string, commit: IGithubResolvedCommit): Promise; + /** + * Drops the remembered answer of one ref only (`key` as for `getResolved`): GitHub has answered that the ref no + * longer exists (or the repository moved), so the earlier answer is of no use. The other refs of the repository + * keep theirs. + */ + dropResolved(key: string): Promise; /** * Drops every remembered answer of one repository, whatever the ref (`repoKey` is `/`, lower case): the * repository has changed under it (a project was created in it). Files and listings are keyed by commit and never @@ -47,6 +53,7 @@ export const NO_GITHUB_FILE_STORE: IGithubFileStore = { putFile: () => Promise.resolve(), getResolved: () => Promise.resolve(undefined), putResolved: () => Promise.resolve(), + dropResolved: () => Promise.resolve(), forgetResolved: () => Promise.resolve(), }; @@ -69,13 +76,25 @@ export function createLazyGithubFileStore( get().then((s) => s.putFile(commitKey, path, file)), getResolved: (key) => get().then((s) => s.getResolved(key)), putResolved: (key, commit) => get().then((s) => s.putResolved(key, commit)), + dropResolved: (key) => get().then((s) => s.dropResolved(key)), forgetResolved: (repoKey) => get().then((s) => s.forgetResolved(repoKey)), }; } -/** How long a call to the cache may take before the cache is given up for the rest of the visit. */ +/** + * How long a read waits for one call to the cache. Past it the read goes on from the network, as it would with an + * empty cache; whether the cache is given up on is decided by {@link guardGithubFileStore}. + */ export const GITHUB_STORE_TIMEOUT_MS = 1500; +/** + * How long the cache has to answer its first call at all before it is given up on for the visit. The first call + * carries the loading of the cache's own chunk and the opening of the database, which on a slow device takes longer + * than a read can be held up for ({@link GITHUB_STORE_TIMEOUT_MS}): reads do not wait for it, but an answer that + * arrives within this limit switches the cache back on for the calls that come after. + */ +export const GITHUB_STORE_OPEN_TIMEOUT_MS = 5000; + /** Starts a timer; returns what cancels it. Injected (see `GITHUB_TIMER`) so a test fires it by hand. */ export type GithubSetTimer = (fire: () => void, ms: number) => () => void; @@ -86,25 +105,67 @@ export const setGithubTimer: GithubSetTimer = (fire, ms) => { /** * The cache as the reader uses it: a call that fails, or does not answer within `timeoutMs`, answers as an empty cache - * would (nothing found, nothing kept). The first call that times out turns the cache off for the rest of the visit, so - * a database that hangs (blocked by another tab, a browser that never answers) costs one wait, not one per file. A - * late answer is ignored. + * would (nothing found, nothing kept); a late answer is never applied to a call that has already completed. + * + * What a call that does not answer in time means depends on what is known of the cache: + * - Not heard from yet (the first call, which loads the cache and opens its database): the calls that follow answer + * empty at once instead of each waiting in turn; the cache is on again as soon as any call is answered, and is + * given up on for the rest of the visit when none has been within `openTimeoutMs` of the first call. + * - Known to work: a database that now hangs (blocked by another tab, a browser that never answers) turns the cache + * off for the rest of the visit, so it costs one wait, not one per file. Only a call that began once the cache was + * known to work can say so: the wait of a call that began before (while the cache was still opening) ends with an + * empty answer for that call and changes nothing. + * A call made `through` the guard (the deletion of a remembered answer, which must reach the database whenever it + * can) is attempted even when the cache is off or paused, with the same wait. */ export function guardGithubFileStore( store: IGithubFileStore, setTimer: GithubSetTimer = setGithubTimer, timeoutMs: number = GITHUB_STORE_TIMEOUT_MS, + openTimeoutMs: number = GITHUB_STORE_OPEN_TIMEOUT_MS, ): IGithubFileStore { - let off = false; - const guarded = (call: () => Promise, empty: T): Promise => { - if (off) { + type State = 'unproven' | 'proven' | 'paused' | 'off'; + let state: State = 'unproven'; + let openLimit: (() => void) | undefined; + let openLimitStarted = false; + + /** The database answered something: it works. Too late to matter once it has been given up on. */ + const answered = (): void => { + openLimit?.(); + openLimit = undefined; + if (state === 'unproven' || state === 'paused') { + state = 'proven'; + } + }; + + const guarded = ( + call: () => Promise, + empty: T, + through = false, + ): Promise => { + if ((state === 'off' || state === 'paused') && !through) { return Promise.resolve(empty); } + // Whether the cache was known to work when this call began: the wait of a call that began before says nothing about + // a cache proven since (a state never goes back to unproven, so for the opening it needs no such check). + const beganProven = state === 'proven'; return new Promise((resolve) => { const cancel = setTimer(() => { - off = true; + if (state === 'unproven') { + state = 'paused'; + } else if (state === 'proven' && beganProven) { + state = 'off'; + } resolve(empty); }, timeoutMs); + if (state === 'unproven' && !openLimitStarted) { + openLimitStarted = true; + // Cancelled by the first answer, so it fires only when the cache never answered. + openLimit = setTimer(() => { + openLimit = undefined; + state = 'off'; + }, openTimeoutMs); + } let answer: Promise; try { answer = call(); @@ -115,10 +176,12 @@ export function guardGithubFileStore( (value) => { cancel(); resolve(value); + answered(); }, () => { cancel(); resolve(empty); + answered(); }, ); }); @@ -131,7 +194,9 @@ export function guardGithubFileStore( getResolved: (key) => guarded(() => store.getResolved(key), undefined), putResolved: (key, commit) => guarded(() => store.putResolved(key, commit), undefined), + dropResolved: (key) => + guarded(() => store.dropResolved(key), undefined, true), forgetResolved: (repoKey) => - guarded(() => store.forgetResolved(repoKey), undefined), + guarded(() => store.forgetResolved(repoKey), undefined, true), }; } diff --git a/libs/datatug/main/src/lib/services/repo/github/github-file-store.spec.ts b/libs/datatug/main/src/lib/services/repo/github/github-file-store.spec.ts index 7ff8ac76..d5ab63b4 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-file-store.spec.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-file-store.spec.ts @@ -337,6 +337,18 @@ describe('the bounds of the cache (design 3.6): 20 MB and 20 repos, two commits expect(await store.getResolved('p/a@HEAD')).toBeDefined(); }); + it('drops the remembered answer of one ref only, not the other refs of the repository', async () => { + const { store } = open({ ...LIMITS, maxResolved: 10 }); + for (const key of ['o/a@HEAD', 'o/a@feature', 'o/ab@feature']) { + await store.putResolved(key, { sha: 's', at: 1 }); + } + await store.dropResolved('o/a@feature'); + await store.dropResolved('o/a@never-there'); + expect(await store.getResolved('o/a@feature')).toBeUndefined(); + expect(await store.getResolved('o/a@HEAD')).toBeDefined(); + expect(await store.getResolved('o/ab@feature')).toBeDefined(); + }); + it('a store that fails to load is no store (the lazy loader)', async () => { const lazy = createLazyGithubFileStore(() => Promise.reject(new Error('chunk failed to load')), @@ -345,6 +357,7 @@ describe('the bounds of the cache (design 3.6): 20 MB and 20 repos, two commits expect(await lazy.getFile(A1, 'f')).toBeUndefined(); expect(await lazy.getResolved('k')).toBeUndefined(); await lazy.putResolved('k', { sha: 's', at: 1 }); + await lazy.dropResolved('k'); await lazy.forgetResolved('o/r'); }); }); diff --git a/libs/datatug/main/src/lib/services/repo/github/github-file-store.ts b/libs/datatug/main/src/lib/services/repo/github/github-file-store.ts index 41d8019b..4754d04e 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-file-store.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-file-store.ts @@ -308,6 +308,13 @@ export function openGithubFileStore( return row ? { sha: row.sha, at: row.at } : undefined; }), + dropResolved: (key) => + run(undefined, async (database) => { + const tx = database.transaction(RESOLVED, 'readwrite'); + tx.objectStore(RESOLVED).delete(key); + await done(tx); + }), + forgetResolved: (repoKey) => run(undefined, async (database) => { const tx = database.transaction(RESOLVED, 'readwrite'); diff --git a/libs/datatug/main/src/lib/services/repo/github/github-project-create.service.spec.ts b/libs/datatug/main/src/lib/services/repo/github/github-project-create.service.spec.ts index dd99033f..2ceea260 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-project-create.service.spec.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-project-create.service.spec.ts @@ -9,6 +9,7 @@ import { ErrorLogger } from '@sneat/core'; import { of, throwError } from 'rxjs'; import { DatatugStoreGithubService } from '../datatug-store.service.github'; +import { GithubRepoError } from './github-api'; import { GithubProjectReaderService } from './github-project-reader.service'; import { DEFAULT_GITHUB_PROJECT_FOLDER, @@ -321,6 +322,24 @@ describe('GithubProjectCreateService', () => { ]); }); + it('commits nothing when GitHub does not say which branch is the default one, and says so (no assumed main)', () => { + const errors: Error[] = []; + service + .createProject( + { org: 'datatug', repo: 'demo-projects', title: 'My project' }, + TOKEN, + ) + .subscribe({ error: (e) => errors.push(e) }); + httpMock + .expectOne((r) => r.method === 'GET' && r.url === REPO_URL) + .flush({ full_name: 'datatug/demo-projects' }); + expect(errors.map((e) => e.message)).toEqual([ + 'GitHub did not return the default branch of datatug/demo-projects, so DataTug cannot tell which branch to commit to.', + ]); + expect(errors[0]).toBeInstanceOf(GithubRepoError); + httpMock.expectNone((r) => r.method === 'PUT'); + }); + it('once the files are committed, the reader and the summary service forget the repository, before the project is registered or opened', async () => { const seen: string[] = []; service diff --git a/libs/datatug/main/src/lib/services/repo/github/github-project-create.service.ts b/libs/datatug/main/src/lib/services/repo/github/github-project-create.service.ts index ec37ac4b..6ad6ac46 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-project-create.service.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-project-create.service.ts @@ -9,7 +9,7 @@ import { GITHUB_API_BASE, IGithubRepoWire, githubApiHeaders, - toGithubRepo, + requireGithubRepo, } from './github-api'; import { GithubProjectReaderService, @@ -180,15 +180,7 @@ export class GithubProjectCreateService { headers: githubApiHeaders(token), }) .pipe( - map((wire) => { - const found = toGithubRepo(wire); - if (!found) { - throw new Error( - `GitHub did not return the repository ${org}/${repo}`, - ); - } - return found.defaultBranch; - }), + map((wire) => requireGithubRepo(wire, `${org}/${repo}`).defaultBranch), ); } diff --git a/libs/datatug/main/src/lib/services/repo/github/github-project-reader.commit.spec.ts b/libs/datatug/main/src/lib/services/repo/github/github-project-reader.commit.spec.ts index c70e427f..df784907 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-project-reader.commit.spec.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-project-reader.commit.spec.ts @@ -370,6 +370,10 @@ describe('GithubProjectReaderService: the commit, the cache, the failover (desig 'demo-project-1/entities/Track/Track.entity.json': '{}', }), ); + // An optional file that is not there is probed in the middle of the visit: it is absent, and nothing moves. + expect( + await first(reader.getRawText(DEMO_ID, 'widgets/none.json')), + ).toBeUndefined(); const entities = ( await first(reader.listDirectory(DEMO_ID, 'entities')) ).map((e) => e.name); @@ -764,6 +768,7 @@ describe('GithubProjectReaderService: the commit, the cache, the failover (desig putFile: () => Promise.reject(new Error('blocked')), getResolved: () => Promise.reject(new Error('blocked')), putResolved: () => Promise.reject(new Error('blocked')), + dropResolved: () => Promise.reject(new Error('blocked')), forgetResolved: () => Promise.reject(new Error('blocked')), }); await readLikeThePages(reader); diff --git a/libs/datatug/main/src/lib/services/repo/github/github-project-reader.followup.spec.ts b/libs/datatug/main/src/lib/services/repo/github/github-project-reader.followup.spec.ts new file mode 100644 index 00000000..d872fc95 --- /dev/null +++ b/libs/datatug/main/src/lib/services/repo/github/github-project-reader.followup.spec.ts @@ -0,0 +1,1469 @@ +import 'fake-indexeddb/auto'; +import { IDBFactory } from 'fake-indexeddb'; +import { TestBed } from '@angular/core/testing'; +import { firstValueFrom } from 'rxjs'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { FakeGithub, ManualTimers, fakeSha } from './github-fake-backend.test'; +import { + GITHUB_RESOLVE_TTL_MS, + GITHUB_STORE_OPEN_TIMEOUT_MS, + GITHUB_STORE_TIMEOUT_MS, + type IGithubFileStore, + type IGithubResolvedCommit, + type IGithubStoredFile, +} from './github-file-store-api'; +import { openGithubFileStore } from './github-file-store'; +import { DatatugStoreGithubService } from '../datatug-store.service.github'; +import { + GITHUB_CLOCK, + GITHUB_FETCH, + GITHUB_FILE_STORE, + GITHUB_TIMER, + GithubProjectReaderService, + GithubReadError, +} from './github-project-reader.service'; +import { + GITHUB_RATE_LIMIT_MESSAGE, + GITHUB_RESOLVE_TIMEOUT_MS, +} from './github-read-limits'; + +// The follow-up of the second review of the GitHub reader (datatug-apps#180, review of #181): what a 404 at a +// remembered commit proves, and what one visit may serve (design `demo-as-github-project.md` 4.5, steps 2 and 5). + +const REPO = 'datatug/projects'; +const ID_B = 'projects@datatug@b'; +const DEMO_REPO = 'datatug/datatug-demo-projects'; +const DEMO_ID = 'datatug-demo-projects@datatug@demo-project-1'; +const SHA_1 = fakeSha(0x111); +const SHA_2 = fakeSha(0x222); +const ENV = 'environments/local/local.env.json'; +const REPO_ENV = `demo-project-1/${ENV}`; + +const first = (o: Parameters>[0]) => + firstValueFrom(o); + +/** One browser: its clock, its IndexedDB, its timers and the fake GitHub; `load()` is a page load. */ +class Browser { + time = 10_000_000; + readonly idb = new IDBFactory(); + readonly timers = new ManualTimers(); + readonly store: IGithubFileStore = openGithubFileStore( + this.idb, + () => this.time, + ); + + constructor(readonly gh: FakeGithub) {} + + load( + store: IGithubFileStore = this.store, + fetch = this.gh.fetch, + ): GithubProjectReaderService { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + { provide: GITHUB_FETCH, useValue: fetch }, + { provide: GITHUB_CLOCK, useValue: () => this.time }, + { provide: GITHUB_FILE_STORE, useValue: store }, + { provide: GITHUB_TIMER, useValue: this.timers.set }, + ], + }); + return TestBed.inject(GithubProjectReaderService); + } +} + +/** A cache in memory that records what was asked of it. */ +class MemoryStore implements IGithubFileStore { + readonly files = new Map(); + readonly resolved = new Map(); + readonly calls: string[] = []; + + getFile(commitKey: string, path: string) { + this.calls.push(`getFile ${commitKey}/${path}`); + return Promise.resolve(this.files.get(`${commitKey}/${path}`)); + } + putFile(commitKey: string, path: string, file: IGithubStoredFile) { + this.calls.push(`putFile ${commitKey}/${path}`); + this.files.set(`${commitKey}/${path}`, file); + return Promise.resolve(); + } + getResolved(key: string) { + this.calls.push(`getResolved ${key}`); + return Promise.resolve(this.resolved.get(key)); + } + putResolved(key: string, commit: IGithubResolvedCommit) { + this.calls.push(`putResolved ${key}`); + this.resolved.set(key, commit); + return Promise.resolve(); + } + dropResolved(key: string) { + this.calls.push(`dropResolved ${key}`); + this.resolved.delete(key); + return Promise.resolve(); + } + forgetResolved(repoKey: string) { + this.calls.push(`forgetResolved ${repoKey}`); + for (const key of [...this.resolved.keys()]) { + if (key.startsWith(`${repoKey}@`)) { + this.resolved.delete(key); + } + } + return Promise.resolve(); + } +} + +function demoFiles(extra: Record = {}): Record { + return { + 'demo-project-1/datatug-project.json': '{"id":"datatug-demo-project"}', + [REPO_ENV]: '{"v":1}', + 'demo-project-1/entities/Album/Album.entity.json': '{}', + ...extra, + }; +} + +const apiUrl = (path: string, repo = DEMO_REPO) => + `https://api.github.com/repos/${repo}/${path}`; +const rawUrl = (sha: string, path: string, repo = DEMO_REPO) => + `https://raw.githubusercontent.com/${repo}/${sha}/${path}`; +const mirrorUrl = (sha: string | undefined, path: string, repo = DEMO_REPO) => + `https://cdn.jsdelivr.net/gh/${repo}${sha ? `@${sha}` : ''}/${path}`; + +/** A request that never answers, until the caller gives up on it (as `fetch` does). */ +const hangUntilAborted = (init: RequestInit): Promise => + new Promise((_resolve, reject) => { + init.signal?.addEventListener('abort', () => + reject(new TypeError('aborted')), + ); + }); + +/** A fetch that holds the requests `holds` says to, until `release()`; `reached` settles when the first is held. */ +function gated( + gh: FakeGithub, + holds: (url: string) => boolean, +): { + fetch: typeof gh.fetch; + reached: Promise; + release: () => void; +} { + let release: () => void = () => undefined; + const open = new Promise((resolve) => (release = resolve)); + let reach: () => void = () => undefined; + const reached = new Promise((resolve) => (reach = resolve)); + return { + fetch: async (url, init) => { + if (holds(url)) { + reach(); + await open; + } + return gh.fetch(url, init); + }, + reached, + release, + }; +} + +describe('GithubProjectReaderService: the follow-up of the second review', () => { + let gh: FakeGithub; + let browser: Browser; + + beforeEach(() => { + gh = new FakeGithub(); + gh.addRepo(DEMO_REPO, SHA_1, demoFiles()); + browser = new Browser(gh); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + /** A first visit: the project file, the listing and a file, all kept under SHA_1, and the answer remembered. */ + const firstVisit = async (): Promise => { + const reader = browser.load(); + await first(reader.getRawJson(DEMO_ID, 'datatug-project.json')); + await first(reader.listDirectory(DEMO_ID, 'entities')); + await first(reader.getRawJson(DEMO_ID, ENV)); + }; + + /** The project moves on: a push that changes what every kind of read would say. */ + const push = (): void => + gh.push( + DEMO_REPO, + SHA_2, + demoFiles({ + 'demo-project-1/datatug-project.json': '{"id":"new"}', + [REPO_ENV]: '{"v":2}', + 'demo-project-1/entities/Track/Track.entity.json': '{}', + }), + ); + + describe('A1: an absent file at a remembered commit is an absent file', () => { + it.each([ + ['after the project file and the listing', false], + ['before anything else', true], + ])( + 'a memory under 5 minutes old, a push, one probe of an absent file %s: the whole visit stays at the one commit', + async (_name, probeFirst) => { + await firstVisit(); + push(); + browser.time += 60_000; + gh.reset(); + + const reader = browser.load(); + const probe = () => + first(reader.getRawText(DEMO_ID, 'widgets/none.json')); + if (probeFirst) { + expect(await probe()).toBeUndefined(); + } + const project = await first( + reader.getRawJson(DEMO_ID, 'datatug-project.json'), + ); + const entities = ( + await first(reader.listDirectory(DEMO_ID, 'entities')) + ).map((e) => e.name); + if (!probeFirst) { + expect(await probe()).toBeUndefined(); + } + const env = await first(reader.getRawJson(DEMO_ID, ENV)); + const album = await first( + reader.getRawJson(DEMO_ID, 'entities/Album/Album.entity.json'), + ); + const other = await first( + reader.getRawText(DEMO_ID, 'boards/b/board.json'), + ); + + // Every read of the visit is of SHA_1: the project file, the listing, the files, and what readInfo says. + expect(project).toEqual({ id: 'datatug-demo-project' }); + expect(entities).toEqual(['Album']); + expect(env).toEqual({ v: 1 }); + expect(album).toEqual({}); + expect(other).toBeUndefined(); + expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ + state: 'resolved', + commit: SHA_1, + mayBeStale: false, + }); + // Nothing asked of the API (no commit to look up, no listing), and the probes went to the one commit. + expect(gh.urls('api.github.com')).toEqual([]); + for (const url of gh.urls('raw.githubusercontent.com')) { + expect(url).toContain(`/${SHA_1}/`); + } + expect( + await browser.store.getResolved(`${DEMO_REPO}@HEAD`), + ).toMatchObject({ sha: SHA_1 }); + }, + ); + + it('a push in the middle of a visit that began from a memory changes nothing of it, an absent file probed included; the next visit sees the new commit', async () => { + await firstVisit(); + browser.time += 60_000; + gh.reset(); + const reader = browser.load(); + await first(reader.getRawText(DEMO_ID, 'datatug-project.json')); + push(); + expect( + await first(reader.getRawText(DEMO_ID, 'widgets/none.json')), + ).toBeUndefined(); + expect( + (await first(reader.listDirectory(DEMO_ID, 'entities'))).map( + (e) => e.name, + ), + ).toEqual(['Album']); + expect(await first(reader.getRawJson(DEMO_ID, ENV))).toEqual({ v: 1 }); + expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ + commit: SHA_1, + }); + expect(gh.urls('api.github.com')).toEqual([]); + for (const url of gh.urls('raw.githubusercontent.com')) { + expect(url).toContain(`/${SHA_1}/`); + } + + browser.time += GITHUB_RESOLVE_TTL_MS + 1; + const next = browser.load(); + expect(await first(next.getRawJson(DEMO_ID, ENV))).toEqual({ v: 2 }); + }); + + it('API refusing, a remembered commit, a cached listing, an absent-file probe: the memory is kept, and the next page load still renders from the cache', async () => { + await firstVisit(); + const remembered = await browser.store.getResolved(`${DEMO_REPO}@HEAD`); + browser.time += GITHUB_RESOLVE_TTL_MS + 1; + gh.fail.api = 403; + gh.reset(); + + const reader = browser.load(); + expect( + (await first(reader.listDirectory(DEMO_ID, 'entities'))).map( + (e) => e.name, + ), + ).toEqual(['Album']); + expect( + await first(reader.getRawText(DEMO_ID, 'widgets/none.json')), + ).toBeUndefined(); + expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ + state: 'remembered', + commit: SHA_1, + mayBeStale: true, + }); + expect(await browser.store.getResolved(`${DEMO_REPO}@HEAD`)).toEqual( + remembered, + ); + // Asked of GitHub: the one question (refused) and the file host for the probe. + expect(gh.urls()).toEqual([ + apiUrl('commits/HEAD'), + rawUrl(SHA_1, 'demo-project-1/widgets/none.json'), + ]); + + // The next page load, the API still refusing: still the remembered commit, still the cache. + gh.reset(); + const next = browser.load(); + expect( + (await first(next.listDirectory(DEMO_ID, 'entities'))).map( + (e) => e.name, + ), + ).toEqual(['Album']); + expect(await first(next.getRawJson(DEMO_ID, ENV))).toEqual({ v: 1 }); + expect(await first(next.readInfo(DEMO_ID))).toMatchObject({ + state: 'remembered', + commit: SHA_1, + }); + expect(gh.urls()).toEqual([apiUrl('commits/HEAD')]); + }); + + it('a repo with five absent optional files costs no API call on a warm visit within 5 minutes, even when the absent files are new', async () => { + const cold = browser.load(); + await first(cold.getRawJson(DEMO_ID, 'datatug-project.json')); + await first(cold.listDirectory(DEMO_ID, '')); + expect(gh.count('api.github.com')).toBe(2); // cold: the commit and the listing + + browser.time += 60_000; + gh.reset(); + const warm = browser.load(); + await first(warm.getRawJson(DEMO_ID, 'datatug-project.json')); + for (const name of ['a', 'b', 'c', 'd', 'e']) { + expect( + await first(warm.getRawJson(DEMO_ID, `widgets/${name}.json`)), + ).toBeUndefined(); + } + expect(gh.count('api.github.com')).toBe(0); + expect(gh.count('raw.githubusercontent.com')).toBe(5); + + // And the next warm visit asks for nothing at all, the absent files being remembered. + gh.reset(); + const again = browser.load(); + for (const name of ['a', 'b', 'c', 'd', 'e']) { + await first(again.getRawJson(DEMO_ID, `widgets/${name}.json`)); + } + expect(gh.count()).toBe(0); + }); + + describe('what is doubted, and when', () => { + beforeEach(() => { + gh.addRepo(REPO, SHA_1, { + 'a/datatug-project.json': '{}', + 'b/x.json': '{"x":1}', + }); + }); + const remember = (): Promise => + browser.store.putResolved(`${REPO}@HEAD`, { + sha: SHA_1, + at: browser.time, + }); + + it('the project file is 404 and nothing has been read at the commit: asked once more, the commit stands, no second question', async () => { + await remember(); + const reader = browser.load(); + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toBeUndefined(); + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toBeUndefined(); + expect(gh.urls()).toEqual([ + rawUrl(SHA_1, 'b/datatug-project.json', REPO), + apiUrl('commits/HEAD', REPO), + ]); + expect(await first(reader.readInfo(ID_B))).toMatchObject({ + state: 'resolved', + commit: SHA_1, + }); + }); + + it.each([ + [ + 'a file read from the network', + async (reader: GithubProjectReaderService) => + expect(await first(reader.getRawJson(ID_B, 'x.json'))).toEqual({ + x: 1, + }), + [rawUrl(SHA_1, 'b/x.json', REPO)], + ], + [ + 'the listing read from the network', + async (reader: GithubProjectReaderService) => + expect( + (await first(reader.listDirectory(ID_B, ''))).map((e) => e.name), + ).toEqual(['x.json']), + [apiUrl(`git/trees/${SHA_1}?recursive=1`, REPO)], + ], + ])( + 'the project file is 404 after %s at the commit: asked once more all the same, the commit stands', + async (_name, readBefore, urlsBefore) => { + await remember(); + const reader = browser.load(); + await readBefore(reader); + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toBeUndefined(); + expect(gh.urls()).toEqual([ + ...urlsBefore, + rawUrl(SHA_1, 'b/datatug-project.json', REPO), + apiUrl('commits/HEAD', REPO), + ]); + expect(await first(reader.readInfo(ID_B))).toMatchObject({ + state: 'resolved', + commit: SHA_1, + mayBeStale: false, + }); + }, + ); + + it('the project file is 404 after a file and the listing came from the cache: asked once more all the same', async () => { + const warm = browser.load(); + await first(warm.getRawJson(ID_B, 'x.json')); + await first(warm.listDirectory(ID_B, '')); + gh.reset(); + const reader = browser.load(); + expect(await first(reader.getRawJson(ID_B, 'x.json'))).toEqual({ + x: 1, + }); + expect((await first(reader.listDirectory(ID_B, ''))).length).toBe(1); + expect(gh.urls()).toEqual([]); + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toBeUndefined(); + expect(gh.urls()).toEqual([ + rawUrl(SHA_1, 'b/datatug-project.json', REPO), + apiUrl('commits/HEAD', REPO), + ]); + }); + + it('a file that is cached as absent does not make the project file any less doubted', async () => { + await browser.store.putFile(`${REPO}@${SHA_1}`, 'b/nothing.json', { + text: null, + bytes: 0, + }); + await remember(); + const reader = browser.load(); + expect( + await first(reader.getRawJson(ID_B, 'nothing.json')), + ).toBeUndefined(); + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toBeUndefined(); + expect(gh.urls()).toEqual([ + rawUrl(SHA_1, 'b/datatug-project.json', REPO), + apiUrl('commits/HEAD', REPO), + ]); + }); + + it('asked once per commit in a visit: the project file of another folder of the repository, 404 too, is not asked about again', async () => { + await remember(); + const reader = browser.load(); + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toBeUndefined(); + expect( + await first( + reader.getRawJson('projects@datatug@c', 'datatug-project.json'), + ), + ).toBeUndefined(); + expect(gh.urls('api.github.com')).toEqual([ + apiUrl('commits/HEAD', REPO), + ]); + }); + + it('a project pushed from elsewhere into a repository already viewed opens on the first try, for one API call (a regression against the reader before the commits)', async () => { + const viewed = 'projects@datatug@a'; + await first(browser.load().getRawJson(viewed, 'datatug-project.json')); + await first(browser.load().listDirectory(viewed, '')); + gh.push(REPO, SHA_2, { + 'a/datatug-project.json': '{}', + 'b/x.json': '{"x":1}', + 'b/datatug-project.json': '{"id":"new"}', + }); + browser.time += 30_000; + gh.reset(); + + const reader = browser.load(); + // The page of the project that was viewed (all from the cache), then the new one. + expect( + await first(reader.getRawJson(viewed, 'datatug-project.json')), + ).toEqual({}); + expect((await first(reader.listDirectory(viewed, ''))).length).toBe(1); + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toEqual({ id: 'new' }); + + expect(gh.urls('api.github.com')).toEqual([ + apiUrl('commits/HEAD', REPO), + ]); + expect(await first(reader.readInfo(ID_B))).toMatchObject({ + state: 'resolved', + commit: SHA_2, + }); + }); + + it('a commit GitHub answered for in this visit is not doubted', async () => { + const reader = browser.load(); + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toBeUndefined(); + expect(gh.urls()).toEqual([ + apiUrl('commits/HEAD', REPO), + rawUrl(SHA_1, 'b/datatug-project.json', REPO), + ]); + }); + }); + }); + + describe('a remembered commit that is gone', () => { + beforeEach(async () => { + // An earlier visit remembered SHA_1 and kept its environment file; the repository was then force-pushed. + await first(browser.load().getRawJson(DEMO_ID, ENV)); + gh.rewrite( + DEMO_REPO, + SHA_2, + demoFiles({ + 'demo-project-1/datatug-project.json': '{"id":"new"}', + [REPO_ENV]: '{"v":2}', + 'demo-project-1/entities/Track/Track.entity.json': '{}', + }), + ); + gh.reset(); + }); + + it('a listing answered 422 for it: recovers with one re-resolve, and the listing is the new commit’s', async () => { + gh.fail.api = (url) => + url.pathname.endsWith(`/git/trees/${SHA_1}`) ? 422 : undefined; + const reader = browser.load(); + expect( + (await first(reader.listDirectory(DEMO_ID, 'entities'))).map( + (e) => e.name, + ), + ).toEqual(['Album', 'Track']); + expect(gh.urls('api.github.com')).toEqual([ + apiUrl(`git/trees/${SHA_1}?recursive=1`), + apiUrl('commits/HEAD'), + apiUrl(`git/trees/${SHA_2}?recursive=1`), + ]); + expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ + state: 'resolved', + commit: SHA_2, + }); + }); + + it('the visit moves whole: nothing read at the old commit is served afterwards, and readInfo says the commit in use', async () => { + const reader = browser.load(); + // Read at the old commit, out of the cache, before anything doubts it. + expect(await first(reader.getRawJson(DEMO_ID, ENV))).toEqual({ v: 1 }); + expect(await first(reader.getRawText(DEMO_ID, ENV))).toBe('{"v":1}'); + // The listing is refused as unknown: the commit is asked again, and it moved. + expect( + (await first(reader.listDirectory(DEMO_ID, 'entities'))).map( + (e) => e.name, + ), + ).toEqual(['Album', 'Track']); + + expect(await first(reader.getRawJson(DEMO_ID, ENV))).toEqual({ v: 2 }); + expect(await first(reader.getRawText(DEMO_ID, ENV))).toBe('{"v":2}'); + expect( + await first(reader.getRawJson(DEMO_ID, 'datatug-project.json')), + ).toEqual({ id: 'new' }); + expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ + state: 'resolved', + commit: SHA_2, + }); + expect( + gh.urls('api.github.com').filter((u) => u.includes('/commits/')), + ).toEqual([apiUrl('commits/HEAD')]); + }); + + it('the re-resolve is refused: the remembered commit is kept for the visit, the project file is absent, the listing fails with the rate-limit error (not an empty list), and nothing is persisted', async () => { + gh.fail.api = (url) => + url.pathname.includes('/commits/') ? 403 : undefined; + const remembered = await browser.store.getResolved(`${DEMO_REPO}@HEAD`); + const reader = browser.load(); + expect( + await first(reader.getRawJson(DEMO_ID, 'datatug-project.json')), + ).toBeUndefined(); + await expect( + first(reader.listDirectory(DEMO_ID, 'entities')), + ).rejects.toThrow(GITHUB_RATE_LIMIT_MESSAGE); + + // Every read goes to the one remembered commit, never to `HEAD`. + expect(gh.urls()).toEqual([ + rawUrl(SHA_1, 'demo-project-1/datatug-project.json'), + apiUrl('commits/HEAD'), + apiUrl(`git/trees/${SHA_1}?recursive=1`), + apiUrl('commits/HEAD'), // an answer that was not given is not remembered: asked again + ]); + expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ + state: 'resolved', + commit: SHA_1, + }); + expect(await browser.store.getResolved(`${DEMO_REPO}@HEAD`)).toEqual( + remembered, + ); + expect( + await browser.store.getFile( + `${DEMO_REPO}@${SHA_1}`, + 'demo-project-1/datatug-project.json', + ), + ).toBeUndefined(); + }); + + it.each([ + ['refused', 403, GITHUB_RATE_LIMIT_MESSAGE], + ['failing', 500, 'the project listing'], + ])( + 'the re-resolve is %s: the listing read fails, and when GitHub answers again in the same visit the next read recovers at the new commit', + async (_name, status, message) => { + gh.fail.api = (url) => + url.pathname.includes('/commits/') ? status : undefined; + const reader = browser.load(); + const failure = await first( + reader.listDirectory(DEMO_ID, 'entities'), + ).then( + () => undefined, + (e: Error) => e, + ); + expect(failure?.message).toContain(message); + if (status === 500) { + expect(failure).toBeInstanceOf(GithubReadError); + } + expect(gh.urls().some((u) => u.includes('/trees/HEAD'))).toBe(false); + + gh.fail.api = undefined; // GitHub answers again, in the same visit + expect( + (await first(reader.listDirectory(DEMO_ID, 'entities'))).map( + (e) => e.name, + ), + ).toEqual(['Album', 'Track']); + expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ + state: 'resolved', + commit: SHA_2, + }); + expect(await first(reader.getRawJson(DEMO_ID, ENV))).toEqual({ v: 2 }); + }, + ); + + it('the re-resolve times out: the listing read fails with the read error, and no fallback to HEAD', async () => { + vi.useFakeTimers(); + const hangingCommits: typeof gh.fetch = (url, init) => + url.includes('/commits/') + ? hangUntilAborted(init) + : gh.fetch(url, init); + const store = new MemoryStore(); + store.resolved.set(`${DEMO_REPO}@HEAD`, { sha: SHA_1, at: browser.time }); + const reader = browser.load(store, hangingCommits); + + const listing = first(reader.listDirectory(DEMO_ID, 'entities')).then( + () => undefined, + (e: Error) => e, + ); + await vi.advanceTimersByTimeAsync(GITHUB_RESOLVE_TIMEOUT_MS); + expect(await listing).toBeInstanceOf(GithubReadError); + expect(gh.urls().some((u) => u.includes('/trees/HEAD'))).toBe(false); + expect(store.resolved.get(`${DEMO_REPO}@HEAD`)).toEqual({ + sha: SHA_1, + at: browser.time, + }); + }); + + it('a re-resolved commit is not marked as remembered: asking about it again is never repeated', async () => { + // SHA_2 does not have the project file of `b` either: 404 there is an absent file, not a reason to ask again. + gh.addRepo(REPO, SHA_1, { 'b/x.json': '{"x":1}' }); + gh.rewrite(REPO, SHA_2, { 'b/x.json': '{"x":2}' }); + await browser.store.putResolved(`${REPO}@HEAD`, { + sha: SHA_1, + at: browser.time, + }); + gh.reset(); + const reader = browser.load(); + expect( + (await first(reader.listDirectory(ID_B, ''))).map((e) => e.name), + ).toEqual(['x.json']); + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toBeUndefined(); + expect(await first(reader.getRawJson(ID_B, 'x.json'))).toEqual({ x: 2 }); + expect( + gh.urls('api.github.com').filter((u) => u.includes('/commits/')), + ).toEqual([apiUrl('commits/HEAD', REPO)]); + }); + + it('the re-resolve times out: the same', async () => { + vi.useFakeTimers(); + const hangingCommits: typeof gh.fetch = (url, init) => + url.includes('/commits/') + ? hangUntilAborted(init) + : gh.fetch(url, init); + const store = new MemoryStore(); + store.resolved.set(`${DEMO_REPO}@HEAD`, { sha: SHA_1, at: browser.time }); + const reader = browser.load(store, hangingCommits); + + const read = first(reader.getRawJson(DEMO_ID, 'datatug-project.json')); + await vi.advanceTimersByTimeAsync(GITHUB_RESOLVE_TIMEOUT_MS); + expect(await read).toBeUndefined(); + expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ + commit: SHA_1, + }); + expect(store.resolved.get(`${DEMO_REPO}@HEAD`)).toEqual({ + sha: SHA_1, + at: browser.time, + }); + expect(store.calls.filter((c) => c.startsWith('dropResolved'))).toEqual( + [], + ); + }); + }); + + describe('the visit moves while a read is on its way', () => { + it('a read still on its way from the old commit when the visit moves is read again at the new one', async () => { + // SHA_1 is not gone: its listing is refused as unknown (422), so it is doubted, while a file is fetched from it. + push(); + gh.fail.api = (url) => + url.pathname.endsWith(`/git/trees/${SHA_1}`) ? 422 : undefined; + const gate = gated(gh, (url) => url.includes(`/${SHA_1}/${REPO_ENV}`)); + const store = new MemoryStore(); + store.resolved.set(`${DEMO_REPO}@HEAD`, { sha: SHA_1, at: browser.time }); + const reader = browser.load(store, gate.fetch); + + const env = first(reader.getRawJson(DEMO_ID, ENV)); + await gate.reached; + const listing = await first(reader.listDirectory(DEMO_ID, 'entities')); + gate.release(); + + expect(listing.map((e) => e.name)).toEqual(['Album', 'Track']); + expect(await env).toEqual({ v: 2 }); + expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ + state: 'resolved', + commit: SHA_2, + }); + }); + }); + + describe('a move of the visit while other reads are on their way', () => { + // `b` has no project file at SHA_1; a push (SHA_2) adds it, and an entity. + beforeEach(async () => { + gh.addRepo(REPO, SHA_1, { + 'a/datatug-project.json': '{}', + 'b/x.json': '{"x":1}', + }); + gh.push(REPO, SHA_2, { + 'a/datatug-project.json': '{}', + 'b/x.json': '{"x":2}', + 'b/datatug-project.json': '{"id":"b2"}', + 'b/entities/E/E.entity.json': '{}', + }); + await browser.store.putResolved(`${REPO}@HEAD`, { + sha: SHA_1, + at: browser.time, + }); + }); + const names = async ( + reader: GithubProjectReaderService, + ): Promise => + (await first(reader.listDirectory(ID_B, ''))).map((e) => e.name); + + /** Lets whatever can complete without GitHub's answer complete (the cache and the file host answer at once). */ + const settleReads = (): Promise => + new Promise((resolve) => setTimeout(resolve, 60)); + + it('the project file is 404, GitHub is asked which commit, and no other read is delivered at the old commit while the question is on its way: all of it is of the new one (M1)', async () => { + const gate = gated(gh, (url) => url.includes('/commits/')); + const reader = browser.load(browser.store, gate.fetch); + const delivered: string[] = []; + const note = + (name: string) => + (value: T): T => { + delivered.push(name); + return value; + }; + const project = first( + reader.getRawJson(ID_B, 'datatug-project.json'), + ).then(note('project')); + const listing = names(reader).then(note('listing')); + const file = first(reader.getRawJson(ID_B, 'x.json')).then(note('file')); + await gate.reached; // the question about the commit is on its way + await settleReads(); + // The listing and the file have been read at SHA_1; neither is handed to the page, which is about to be told + // that the commit is another. + expect(delivered).toEqual([]); + const info = first(reader.readInfo(ID_B)).then(note('info')); + await settleReads(); + expect(delivered).toEqual([]); // the sources line does not name a commit that is about to be replaced + + gate.release(); + expect(await project).toEqual({ id: 'b2' }); + expect(await listing).toEqual([ + 'datatug-project.json', + 'entities', + 'x.json', + ]); + expect(await file).toEqual({ x: 2 }); + expect(await info).toMatchObject({ state: 'resolved', commit: SHA_2 }); + expect(await names(reader)).toEqual([ + 'datatug-project.json', + 'entities', + 'x.json', + ]); + expect(await first(reader.readInfo(ID_B))).toMatchObject({ + state: 'resolved', + commit: SHA_2, + }); + }); + + it('the same when the question was asked because a listing is 404: a file read from the cache and the project file complete meanwhile, and none is of the old commit', async () => { + // SHA_1 is gone (forced push): its listing is 404, and so is the project file at it, but a file is cached. + gh.rewrite(REPO, SHA_2, { + 'a/datatug-project.json': '{}', + 'b/x.json': '{"x":2}', + 'b/datatug-project.json': '{"id":"b2"}', + 'b/entities/E/E.entity.json': '{}', + }); + await browser.store.putFile(`${REPO}@${SHA_1}`, 'b/x.json', { + text: '{"x":1}', + bytes: 7, + }); + gh.reset(); + const gate = gated(gh, (url) => url.includes('/commits/')); + const reader = browser.load(browser.store, gate.fetch); + const delivered: string[] = []; + const note = + (name: string) => + (value: T): T => { + delivered.push(name); + return value; + }; + const listing = names(reader).then(note('listing')); + const project = first( + reader.getRawJson(ID_B, 'datatug-project.json'), + ).then(note('project')); + const file = first(reader.getRawJson(ID_B, 'x.json')).then(note('file')); + await gate.reached; + await settleReads(); + expect(delivered).toEqual([]); + + gate.release(); + expect(await listing).toEqual([ + 'datatug-project.json', + 'entities', + 'x.json', + ]); + expect(await project).toEqual({ id: 'b2' }); + expect(await file).toEqual({ x: 2 }); + expect(await first(reader.readInfo(ID_B))).toMatchObject({ + commit: SHA_2, + }); + }); + + it('a question that GitHub does not answer holds the reads back for no longer than the time it is given, then they are delivered at the remembered commit', async () => { + vi.useFakeTimers(); + const hangingCommits: typeof gh.fetch = (url, init) => + url.includes('/commits/') + ? hangUntilAborted(init) + : gh.fetch(url, init); + const store = new MemoryStore(); + store.resolved.set(`${REPO}@HEAD`, { sha: SHA_1, at: browser.time }); + const reader = browser.load(store, hangingCommits); + const delivered: string[] = []; + const note = + (name: string) => + (value: T): T => { + delivered.push(name); + return value; + }; + const project = first( + reader.getRawJson(ID_B, 'datatug-project.json'), + ).then(note('project')); + const listing = names(reader).then(note('listing')); + const file = first(reader.getRawJson(ID_B, 'x.json')).then(note('file')); + await vi.advanceTimersByTimeAsync(10); // the question is on its way + const info = first(reader.readInfo(ID_B)).then(note('info')); + await vi.advanceTimersByTimeAsync(GITHUB_RESOLVE_TIMEOUT_MS - 11); + expect(delivered).toEqual([]); // still waiting: the time is not up + + await vi.advanceTimersByTimeAsync(1); + expect(await project).toBeUndefined(); + expect(await listing).toEqual(['x.json']); + expect(await file).toEqual({ x: 1 }); + expect(await info).toMatchObject({ commit: SHA_1 }); + }); + + it('a listing still on its way from the old commit when the visit moves is read again at the new one', async () => { + const gate = gated(gh, (url) => url.includes(`/git/trees/${SHA_1}`)); + const reader = browser.load(browser.store, gate.fetch); + const listing = names(reader); + await gate.reached; + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toEqual({ id: 'b2' }); + + gate.release(); + expect(await listing).toEqual([ + 'datatug-project.json', + 'entities', + 'x.json', + ]); + }); + + it('a file served by the mirror at the old commit is not what the sources line says of the new one', async () => { + gh.fail.raw = 429; + gh.fail.api = (url) => + url.pathname.endsWith(`/git/trees/${SHA_1}`) ? 422 : undefined; + const reader = browser.load(); + expect(await first(reader.getRawJson(ID_B, 'x.json'))).toEqual({ x: 1 }); + expect(await first(reader.readInfo(ID_B))).toMatchObject({ + commit: SHA_1, + fromMirror: true, + }); + + expect(await names(reader)).toEqual([ + 'datatug-project.json', + 'entities', + 'x.json', + ]); + expect(await first(reader.readInfo(ID_B))).toMatchObject({ + commit: SHA_2, + fromMirror: false, + }); + }); + + it('a file from the mirror whose answer comes after the visit moved is not what the sources line says of the new commit', async () => { + gh.fail.raw = (url) => + url.pathname.includes(SHA_1) && url.pathname.endsWith('x.json') + ? 429 + : undefined; + const gate = gated(gh, (url) => url.includes('cdn.jsdelivr.net')); + const reader = browser.load(browser.store, gate.fetch); + const file = first(reader.getRawJson(ID_B, 'x.json')); + await gate.reached; // the mirror is asked for the file at SHA_1 + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toEqual({ id: 'b2' }); // the visit moves to SHA_2 + gate.release(); // the mirror answers for SHA_1, late + + expect(await file).toEqual({ x: 2 }); + expect(await first(reader.readInfo(ID_B))).toMatchObject({ + commit: SHA_2, + fromMirror: false, + }); + }); + + it('a read at the old commit that fails after the visit moved is read again at the new one, not failed with the old one’s error', async () => { + gh.fail.raw = (url) => + url.pathname.includes(SHA_1) && url.pathname.includes('E.entity') + ? 500 + : undefined; + gh.fail.mirror = 500; + const gate = gated( + gh, + (url) => + url.includes('raw.githubusercontent.com') && url.includes('E.entity'), + ); + const reader = browser.load(browser.store, gate.fetch); + const entity = first(reader.getRawJson(ID_B, 'entities/E/E.entity.json')); + await gate.reached; + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toEqual({ id: 'b2' }); + gate.release(); // the read at SHA_1 fails, now that the visit is at SHA_2 + + expect(await entity).toEqual({}); + expect(gh.urls('raw.githubusercontent.com')).toContain( + rawUrl(SHA_2, 'b/entities/E/E.entity.json', REPO), + ); + }); + + it('a read that fails at the commit the visit stays at is a failure, once', async () => { + gh.fail.raw = (url) => + url.pathname.endsWith('x.json') ? 500 : undefined; + gh.fail.mirror = 500; + const reader = browser.load(); + await expect( + first(reader.getRawJson(ID_B, 'x.json')), + ).rejects.toBeInstanceOf(GithubReadError); + expect(gh.urls('raw.githubusercontent.com')).toEqual([ + rawUrl(SHA_1, 'b/x.json', REPO), + ]); + }); + + it('a listing 404 at a commit GitHub answered for in this visit is not doubted', async () => { + await browser.store.dropResolved(`${REPO}@HEAD`); + gh.fail.api = (url) => + url.pathname.includes('/git/trees/') ? 404 : undefined; + const reader = browser.load(); + expect(await names(reader)).toEqual([]); + expect(gh.urls()).toEqual([ + apiUrl('commits/HEAD', REPO), + apiUrl(`git/trees/${SHA_2}?recursive=1`, REPO), + ]); + }); + + it('asked again and GitHub answers, though it did not at first: the sources line no longer says "may not be the latest"', async () => { + browser.time += GITHUB_RESOLVE_TTL_MS + 1; // the memory has expired + let asked = 0; + gh.fail.api = (url) => + url.pathname.includes('/commits/') && asked++ === 0 ? 403 : undefined; + const reader = browser.load(); + // The project file is 404 at the commit that was remembered (SHA_1): the commit is asked again, and it is now + // SHA_2, which has the project file. + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toEqual({ id: 'b2' }); + expect(await first(reader.readInfo(ID_B))).toMatchObject({ + state: 'resolved', + commit: SHA_2, + mayBeStale: false, + }); + }); + + it('asked again and GitHub names the same commit, answering this time: the commit is no longer "remembered"', async () => { + browser.time += GITHUB_RESOLVE_TTL_MS + 1; + gh.rewrite(REPO, SHA_1, { + 'a/datatug-project.json': '{}', + 'b/x.json': '{"x":1}', + }); + let asked = 0; + gh.fail.api = (url) => + url.pathname.includes('/commits/') && asked++ === 0 ? 403 : undefined; + const reader = browser.load(); + expect( + await first(reader.getRawJson(ID_B, 'datatug-project.json')), + ).toBeUndefined(); + expect(await first(reader.readInfo(ID_B))).toMatchObject({ + state: 'resolved', + commit: SHA_1, + mayBeStale: false, + }); + expect(gh.urls()).toEqual([ + apiUrl('commits/HEAD', REPO), + rawUrl(SHA_1, 'b/datatug-project.json', REPO), + apiUrl('commits/HEAD', REPO), + ]); + }); + }); + + describe('the memory is dropped only for the ref concerned, and only once GitHub has answered', () => { + const FEATURE_ID = `${DEMO_ID}@feature`; + + it('the ref no longer exists: its memory is dropped, the memory of the default branch is not', async () => { + const repo = gh.addRepo(DEMO_REPO, SHA_1, demoFiles()); + repo.refs['feature'] = SHA_1; + await first(browser.load().getRawJson(DEMO_ID, ENV)); + await first(browser.load().getRawJson(FEATURE_ID, ENV)); + expect( + await browser.store.getResolved(`${DEMO_REPO}@HEAD`), + ).toBeDefined(); + expect( + await browser.store.getResolved(`${DEMO_REPO}@feature`), + ).toBeDefined(); + + // The branch is deleted and the repository rewritten. + gh.rewrite(DEMO_REPO, SHA_2, demoFiles({ [REPO_ENV]: '{"v":2}' })); + delete repo.refs['feature']; + gh.reset(); + + const reader = browser.load(); + expect( + await first(reader.getRawJson(FEATURE_ID, 'datatug-project.json')), + ).toBeUndefined(); + expect(await first(reader.readInfo(FEATURE_ID))).toMatchObject({ + state: 'missing', + }); + expect(gh.urls()).toEqual([ + rawUrl(SHA_1, 'demo-project-1/datatug-project.json'), + apiUrl('commits/feature'), + ]); + expect( + await browser.store.getResolved(`${DEMO_REPO}@feature`), + ).toBeUndefined(); + expect( + await browser.store.getResolved(`${DEMO_REPO}@HEAD`), + ).toMatchObject({ + sha: SHA_1, + }); + }); + + it('an expired memory of a ref that no longer exists is dropped when GitHub says so', async () => { + const id = `${DEMO_ID}@gone-branch`; + await browser.store.putResolved(`${DEMO_REPO}@gone-branch`, { + sha: SHA_1, + at: browser.time - GITHUB_RESOLVE_TTL_MS - 1, + }); + const reader = browser.load(); + expect( + await first(reader.getRawJson(id, 'datatug-project.json')), + ).toBeUndefined(); + expect(await first(reader.readInfo(id))).toMatchObject({ + state: 'missing', + }); + expect( + await browser.store.getResolved(`${DEMO_REPO}@gone-branch`), + ).toBeUndefined(); + }); + + it('the memory of the ref is not touched before GitHub has answered, and is replaced by the answer', async () => { + gh.rewrite(DEMO_REPO, SHA_2, demoFiles({ [REPO_ENV]: '{"v":2}' })); + const gate = gated(gh, (url) => url.includes('/commits/')); + const store = new MemoryStore(); + store.resolved.set(`${DEMO_REPO}@HEAD`, { sha: SHA_1, at: browser.time }); + const reader = browser.load(store, gate.fetch); + + const read = first(reader.getRawJson(DEMO_ID, 'datatug-project.json')); + await gate.reached; + // The question is on its way: the memory of the earlier answer is still there. + expect(store.resolved.get(`${DEMO_REPO}@HEAD`)).toEqual({ + sha: SHA_1, + at: browser.time, + }); + gate.release(); + expect(await read).toEqual({ id: 'datatug-demo-project' }); + expect(store.resolved.get(`${DEMO_REPO}@HEAD`)).toMatchObject({ + sha: SHA_2, + }); + expect(store.calls.filter((c) => c.startsWith('forgetResolved'))).toEqual( + [], + ); + }); + }); + + describe('forget and a resolve in flight', () => { + it('a resolve that started before forget does not write its commit back afterwards', async () => { + const gate = gated(gh, (url) => url.includes('/commits/')); + const reader = browser.load(browser.store, gate.fetch); + const read = first(reader.getRawText(DEMO_ID, 'datatug-project.json')); + await gate.reached; + + await reader.forget('datatug', 'datatug-demo-projects'); + gate.release(); + expect(await read).toBe('{"id":"datatug-demo-project"}'); + + expect( + await browser.store.getResolved(`${DEMO_REPO}@HEAD`), + ).toBeUndefined(); + // The next read asks again. + gh.reset(); + await first( + reader.getRawText(DEMO_ID, 'environments/local/local.env.json'), + ); + expect(gh.urls('api.github.com')).toEqual([apiUrl('commits/HEAD')]); + }); + + it('a resolve that starts after forget is remembered as usual', async () => { + const reader = browser.load(); + await reader.forget('datatug', 'datatug-demo-projects'); + await first(reader.getRawText(DEMO_ID, 'datatug-project.json')); + expect( + await browser.store.getResolved(`${DEMO_REPO}@HEAD`), + ).toMatchObject({ + sha: SHA_1, + }); + }); + + it('with the cache given up on, forget still attempts the persisted delete', async () => { + const calls: string[] = []; + const hanging: IGithubFileStore = { + getFile: () => new Promise(() => undefined), + putFile: () => new Promise(() => undefined), + getResolved: () => new Promise(() => undefined), + putResolved: () => new Promise(() => undefined), + dropResolved: () => new Promise(() => undefined), + forgetResolved: (repoKey) => { + calls.push(repoKey); + return new Promise(() => undefined); + }, + }; + const reader = browser.load(hanging); + const read = first(reader.getRawText(DEMO_ID, 'datatug-project.json')); + await browser.timers.waitFor(); + browser.timers.fireAll(); // the cache is given up on + await read; + expect(calls).toEqual([]); + + const forgotten = reader.forget('datatug', 'datatug-demo-projects'); + await browser.timers.waitFor(); + expect(calls).toEqual(['datatug/datatug-demo-projects']); + browser.timers.fireAll(); // it does not answer: forget does not wait for it for ever + await expect(forgotten).resolves.toBeUndefined(); + }); + }); + + describe('a cache that opens slowly (the chunk, the database)', () => { + /** The real cache, answering nothing until `open()` (a slow device loading and opening it). */ + const slow = (): { store: IGithubFileStore; open: () => void } => { + let open: () => void = () => undefined; + const opened = new Promise((resolve) => (open = resolve)); + const real = browser.store; + return { + open, + store: { + getFile: (k, p) => opened.then(() => real.getFile(k, p)), + putFile: (k, p, f) => opened.then(() => real.putFile(k, p, f)), + getResolved: (k) => opened.then(() => real.getResolved(k)), + putResolved: (k, c) => opened.then(() => real.putResolved(k, c)), + dropResolved: (k) => opened.then(() => real.dropResolved(k)), + forgetResolved: (k) => opened.then(() => real.forgetResolved(k)), + }, + }; + }; + + it('the first read does not wait for it past the usual wait, and what is read after it opens is kept', async () => { + const { store, open } = slow(); + const reader = browser.load(store); + const project = first(reader.getRawJson(DEMO_ID, 'datatug-project.json')); + await browser.timers.waitFor(2); + browser.timers.fireWith(GITHUB_STORE_TIMEOUT_MS); + expect(await project).toEqual({ id: 'datatug-demo-project' }); + // The opening is still given until its own, longer limit. + expect(browser.timers.count).toBe(1); + expect(browser.timers.delays).toContain(GITHUB_STORE_OPEN_TIMEOUT_MS); + + open(); + await new Promise((resolve) => setTimeout(resolve, 10)); + expect(await first(reader.getRawJson(DEMO_ID, ENV))).toEqual({ v: 1 }); + expect( + await browser.store.getFile(`${DEMO_REPO}@${SHA_1}`, REPO_ENV), + ).toEqual({ text: '{"v":1}', bytes: 7 }); + }); + + it('a cache that has not opened by its own limit is given up on for the visit', async () => { + const { store, open } = slow(); + const reader = browser.load(store); + const project = first(reader.getRawJson(DEMO_ID, 'datatug-project.json')); + await browser.timers.waitFor(2); + browser.timers.fireAll(); + await project; + open(); + await new Promise((resolve) => setTimeout(resolve, 10)); + await first(reader.getRawJson(DEMO_ID, ENV)); + expect( + await browser.store.getFile(`${DEMO_REPO}@${SHA_1}`, REPO_ENV), + ).toBeUndefined(); + }); + }); + + describe('raw refused and the mirror says 404', () => { + it('at a commit: the optional file is absent for this visit, and nothing is kept', async () => { + gh.fail.raw = 429; + const reader = browser.load(); + expect( + await first(reader.getRawJson(DEMO_ID, 'widgets/none.json')), + ).toBeUndefined(); + expect(gh.urls()).toEqual([ + apiUrl('commits/HEAD'), + rawUrl(SHA_1, 'demo-project-1/widgets/none.json'), + mirrorUrl(SHA_1, 'demo-project-1/widgets/none.json'), + ]); + expect( + await browser.store.getFile( + `${DEMO_REPO}@${SHA_1}`, + 'demo-project-1/widgets/none.json', + ), + ).toBeUndefined(); + // Not asked again in this visit. + gh.reset(); + await first(reader.getRawJson(DEMO_ID, 'widgets/none.json')); + expect(gh.count()).toBe(0); + }); + + it('with no commit, an optional file that the file host says is not there is absent, and nothing is kept', async () => { + gh.fail.api = 403; + const reader = browser.load(); + expect( + await first(reader.getRawText(DEMO_ID, 'widgets/none.json')), + ).toBeUndefined(); + expect( + await browser.store.getFile( + `${DEMO_REPO}@HEAD`, + 'demo-project-1/widgets/none.json', + ), + ).toBeUndefined(); + expect(gh.urls('cdn.jsdelivr.net')).toEqual([]); + }); + + it('at a commit given in the id: the same', async () => { + gh.fail.raw = 'network'; + const reader = browser.load(); + expect( + await first( + reader.getRawText(`${DEMO_ID}@${SHA_1}`, 'widgets/none.json'), + ), + ).toBeUndefined(); + }); + + it('the project file is not taken for absent that way: the mirror may lag behind a project just created', async () => { + gh.fail.raw = 429; + const reader = browser.load(); + const error = await first( + reader.getRawText( + `${DEMO_ID}@${SHA_1}`, + 'nothing/datatug-project.json', + ), + ).catch((e) => e); + // (the file is named like the project file of a folder, and is not there) + expect(error).toBeInstanceOf(GithubReadError); + }); + + it('with no commit (unversioned reads of HEAD): still an error naming both hosts', async () => { + gh.fail.api = 403; + gh.fail.raw = 429; + const reader = browser.load(); + const error = await first( + reader.getRawText(DEMO_ID, 'widgets/none.json'), + ).catch((e) => e); + expect(error).toBeInstanceOf(GithubReadError); + expect(gh.urls('cdn.jsdelivr.net')).toEqual([ + mirrorUrl(undefined, 'demo-project-1/widgets/none.json'), + ]); + }); + }); + + describe('the project summary of the store service follows the commit of the reader', () => { + const SUMMARY_FILE = 'demo-project-1/datatug-project.json'; + const titled = (title: string, extra: Record = {}) => + demoFiles({ [SUMMARY_FILE]: `{"title":"${title}"}`, ...extra }); + + it('a summary read at a commit that was only remembered is not kept once the visit has moved to another one', async () => { + gh.addRepo(DEMO_REPO, SHA_1, titled('one')); + const warm = browser.load(); + await first(warm.getRawJson(DEMO_ID, 'datatug-project.json')); + gh.rewrite(DEMO_REPO, SHA_2, titled('two')); + browser.time += 60_000; + gh.reset(); + + const reader = browser.load(); + const summaries = TestBed.inject(DatatugStoreGithubService); + // From the cache, at the remembered commit: the project as it was. + expect(await first(summaries.getProjectSummary(DEMO_ID))).toMatchObject({ + title: 'one', + }); + // The listing says the commit is gone; GitHub names the other one, and the visit moves to it. + expect( + (await first(reader.listDirectory(DEMO_ID, ''))).map((e) => e.name), + ).toEqual(['datatug-project.json', 'entities', 'environments']); + expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ + commit: SHA_2, + }); + + expect(await first(summaries.getProjectSummary(DEMO_ID))).toMatchObject({ + title: 'two', + }); + // And is kept again from there on: one read, whoever asks. + gh.reset(); + await first(summaries.getProjectSummary(DEMO_ID)); + expect(gh.count()).toBe(0); + }); + + it('the epoch of the visit changes when the visit moves, and when the repository is forgotten, and not otherwise', async () => { + gh.addRepo(DEMO_REPO, SHA_1, titled('one')); + await first(browser.load().getRawJson(DEMO_ID, 'datatug-project.json')); + gh.rewrite(DEMO_REPO, SHA_2, titled('two')); + browser.time += 60_000; + const reader = browser.load(); + const before = reader.visitEpoch(DEMO_ID); + await first(reader.getRawJson(DEMO_ID, 'datatug-project.json')); + expect(reader.visitEpoch(DEMO_ID)).toBe(before); // a read at the remembered commit is not a move + await first(reader.listDirectory(DEMO_ID, '')); + const moved = reader.visitEpoch(DEMO_ID); + expect(moved).not.toBe(before); + expect(reader.visitEpoch(DEMO_ID)).toBe(moved); + await reader.forget('datatug', 'datatug-demo-projects'); + expect(reader.visitEpoch(DEMO_ID)).not.toBe(moved); + expect(() => reader.visitEpoch('not-a-project-id')).toThrow(); + + // Forgotten with no move before it: the visit starts afresh, which is a change all the same. + const other = browser.load(); + await first(other.getRawJson(DEMO_ID, 'datatug-project.json')); + const unmoved = other.visitEpoch(DEMO_ID); + await other.forget('datatug', 'datatug-demo-projects'); + expect(other.visitEpoch(DEMO_ID)).not.toBe(unmoved); + }); + }); + + describe('API calls of a visit (design 4.5: one call for the commit, one for the listing)', () => { + const apiCalls = (): number => gh.count('api.github.com'); + /** What the project pages read: the project file, the listing and an environment file. */ + const pages = async (reader: GithubProjectReaderService): Promise => { + await first(reader.getRawJson(DEMO_ID, 'datatug-project.json')); + await first(reader.listDirectory(DEMO_ID, '')); + await first(reader.getRawJson(DEMO_ID, ENV)); + }; + it('cold: 2; warm within 5 minutes: 0; warm after 5 minutes, nothing changed: 1', async () => { + await pages(browser.load()); + expect(apiCalls()).toBe(2); + + browser.time += 60_000; + gh.reset(); + await pages(browser.load()); + expect(apiCalls()).toBe(0); + expect(gh.count()).toBe(0); + + browser.time += GITHUB_RESOLVE_TTL_MS; + gh.reset(); + await pages(browser.load()); + expect(gh.urls()).toEqual([apiUrl('commits/HEAD')]); + }); + + it('the project changed: 2 (the commit and the listing of the new one), and only the files that are read again', async () => { + await pages(browser.load()); + push(); + browser.time += GITHUB_RESOLVE_TTL_MS + 1; + gh.reset(); + await pages(browser.load()); + expect(gh.urls('api.github.com')).toEqual([ + apiUrl('commits/HEAD'), + apiUrl(`git/trees/${SHA_2}?recursive=1`), + ]); + }); + + it('five new absent optional files on a warm visit: 0 API calls', async () => { + await pages(browser.load()); + browser.time += 60_000; + gh.reset(); + const warm = browser.load(); + await pages(warm); + for (const name of ['a', 'b', 'c', 'd', 'e']) { + await first(warm.getRawJson(DEMO_ID, `widgets/${name}.json`)); + } + expect(apiCalls()).toBe(0); + }); + + it('a repository with no project file: 1 call per page load, however warm', async () => { + const bare = 'datatug-demo-projects@datatug@nothing-here'; + const counts: number[] = []; + for (const step of [0, 30_000, 30_000, GITHUB_RESOLVE_TTL_MS]) { + browser.time += step; + gh.reset(); + const reader = browser.load(); + await first(reader.getRawJson(bare, 'datatug-project.json')); + await first(reader.getRawJson(bare, 'datatug-project.json')); + counts.push(apiCalls()); + } + expect(counts).toEqual([1, 1, 1, 1]); + }); + + it('a commit that is gone: 3 (the listing, the commit asked again, the listing of the new one)', async () => { + await first(browser.load().getRawJson(DEMO_ID, ENV)); // the commit remembered, no listing kept + gh.rewrite(DEMO_REPO, SHA_2, demoFiles()); + browser.time += 60_000; + gh.reset(); + await first(browser.load().listDirectory(DEMO_ID, '')); + expect(gh.urls('api.github.com')).toEqual([ + apiUrl(`git/trees/${SHA_1}?recursive=1`), + apiUrl('commits/HEAD'), + apiUrl(`git/trees/${SHA_2}?recursive=1`), + ]); + }); + }); +}); diff --git a/libs/datatug/main/src/lib/services/repo/github/github-project-reader.resilience.spec.ts b/libs/datatug/main/src/lib/services/repo/github/github-project-reader.resilience.spec.ts index 2d8ea975..7326ac29 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-project-reader.resilience.spec.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-project-reader.resilience.spec.ts @@ -251,6 +251,7 @@ describe('GithubProjectReaderService: the review of round 1', () => { putFile: () => new Promise(() => undefined), getResolved: () => new Promise(() => undefined), putResolved: () => new Promise(() => undefined), + dropResolved: () => new Promise(() => undefined), forgetResolved: () => new Promise(() => undefined), }); @@ -453,61 +454,67 @@ describe('GithubProjectReaderService: the review of round 1', () => { ).toEqual([apiUrl('commits/HEAD')]); }); - it('the resolve call is refused when asking again: the read degrades to HEAD, as when nothing is remembered', async () => { + it('the resolve call is refused when asking again: the remembered commit is kept for the visit, and the 404 is absent', async () => { gh.fail.api = (url) => url.pathname.includes('/commits/') ? 403 : undefined; const reader = browser.load(); expect( await first(reader.getRawJson(DEMO_ID, 'datatug-project.json')), - ).toEqual({ id: 'new' }); + ).toBeUndefined(); expect(await first(reader.readInfo(DEMO_ID))).toMatchObject({ - state: 'unresolved', - mayBeStale: true, + state: 'resolved', + commit: SHA_1, }); expect(gh.urls('raw.githubusercontent.com')).toEqual([ rawUrl(SHA_1, 'demo-project-1/datatug-project.json'), - rawUrl('HEAD', 'demo-project-1/datatug-project.json'), ]); }); - it.each([ - [ - 'the project file', - 'b/datatug-project.json', - 'b', - 'datatug-project.json', - ], - ['another file', 'b/widgets/x.json', 'b', 'widgets/x.json'], - ])( - 'the commit is alive and %s really is not there: asks once more, says missing, and does not ask again', - async (_name, repoPath, folder, file) => { - const bare = new FakeGithub(); - bare.addRepo(REPO, SHA_1, { 'a/datatug-project.json': '{}' }); - const other = new Browser(bare); - await first(other.load().listDirectory(ID_A, '')); - bare.reset(); - - const reader = other.load(); - const id = `projects@datatug@${folder}`; - expect(await first(reader.getRawJson(id, file))).toBeUndefined(); - expect(await first(reader.getRawJson(id, file))).toBeUndefined(); - expect(await first(reader.readInfo(id))).toMatchObject({ - state: 'resolved', - commit: SHA_1, - }); - expect(bare.urls()).toEqual([ - rawUrl(SHA_1, repoPath, REPO), - apiUrl('commits/HEAD', REPO), - rawUrl(SHA_1, repoPath, REPO), - ]); - // The project file is the one answer that is not kept: it is about to be created. - expect(await other.store.getFile(`${REPO}@${SHA_1}`, repoPath)).toEqual( - file === 'datatug-project.json' - ? undefined - : { text: null, bytes: 0 }, - ); - }, - ); + it('the commit is alive and the project file really is not there: asks once more, says missing, and does not ask again', async () => { + const bare = new FakeGithub(); + bare.addRepo(REPO, SHA_1, { 'a/datatug-project.json': '{}' }); + const other = new Browser(bare); + await first(other.load().listDirectory(ID_A, '')); + bare.reset(); + + const reader = other.load(); + const id = 'projects@datatug@b'; + expect( + await first(reader.getRawJson(id, 'datatug-project.json')), + ).toBeUndefined(); + expect( + await first(reader.getRawJson(id, 'datatug-project.json')), + ).toBeUndefined(); + expect(await first(reader.readInfo(id))).toMatchObject({ + state: 'resolved', + commit: SHA_1, + }); + expect(bare.urls()).toEqual([ + rawUrl(SHA_1, 'b/datatug-project.json', REPO), + apiUrl('commits/HEAD', REPO), + ]); + // The project file is the one answer that is not kept: it is about to be created. + expect( + await other.store.getFile(`${REPO}@${SHA_1}`, 'b/datatug-project.json'), + ).toBeUndefined(); + }); + + it('the commit is alive and another file is not there: absent, with no question, and remembered as absent', async () => { + const bare = new FakeGithub(); + bare.addRepo(REPO, SHA_1, { 'a/datatug-project.json': '{}' }); + const other = new Browser(bare); + await first(other.load().listDirectory(ID_A, '')); + bare.reset(); + + const reader = other.load(); + expect( + await first(reader.getRawJson('projects@datatug@b', 'widgets/x.json')), + ).toBeUndefined(); + expect(bare.urls()).toEqual([rawUrl(SHA_1, 'b/widgets/x.json', REPO)]); + expect( + await other.store.getFile(`${REPO}@${SHA_1}`, 'b/widgets/x.json'), + ).toEqual({ text: null, bytes: 0 }); + }); it('a commit that GitHub answered for just now is not doubted: a missing project file is missing, with no second question', async () => { const bare = new FakeGithub(); diff --git a/libs/datatug/main/src/lib/services/repo/github/github-project-reader.service.ts b/libs/datatug/main/src/lib/services/repo/github/github-project-reader.service.ts index 889eb021..f6065ab7 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-project-reader.service.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-project-reader.service.ts @@ -327,11 +327,22 @@ function isProjectFile(path: string): boolean { return path === PROJECT_FILE_NAME || path.endsWith(`/${PROJECT_FILE_NAME}`); } -/** What a read answers when the commit it was reading at was found not to exist and has been replaced: read again. */ +/** What a read answers when the visit has moved to another commit while it was asking: read again. */ const STALE = Symbol('stale commit'); +/** How many times a read that failed at an old commit is read again at the new one (the visit moves once, in practice). */ +const MAX_REREADS_AFTER_FAILURE = 3; + +/** + * What asking GitHub again about a remembered commit came to: the visit `moved` to another commit; the commit + * `stands` (GitHub named it again); or GitHub did not answer, `refused` (a limit) or `unanswered` (a failure, a timeout). + */ +type GithubDoubtOutcome = 'moved' | 'stands' | 'refused' | 'unanswered'; + /** A commit, and whether it is what an earlier answer said rather than what GitHub said in this visit. */ interface IGithubResolvedCommit extends IGithubCommitResolution { + /** For `unresolved`: GitHub refused the call (a limit, a block), as opposed to failing or not answering. */ + readonly refused?: boolean; /** * The commit is a remembered answer (of the last 5 minutes, or of an earlier visit when GitHub would not answer), * so it may be one that no longer exists. @@ -357,6 +368,20 @@ function readCachedTree(text: string): IGithubGitTreeEntry[] | undefined { } } +/** A read in progress or finished. `done` is set in the same step as the answer, so a move of the visit is never between. */ +interface IGithubRead { + promise: Promise; + done: boolean; +} + +function startRead( + run: (read: IGithubRead) => Promise, +): IGithubRead { + const read = { done: false } as IGithubRead; + read.promise = run(read); + return read; +} + /** Everything read for one `owner/repo@ref` during this page load: one commit, one listing, the files. */ interface IGithubSession { readonly org: string; @@ -364,11 +389,16 @@ interface IGithubSession { readonly ref?: string; /** Asks again which commit the id names; `fresh` ignores what is remembered. */ readonly resolve: (fresh: boolean) => Promise; - /** The commit every read of the session is at; replaced once, when a remembered commit proves not to exist. */ + /** + * The commit every read of the session is at. Replaced when GitHub, asked again about a remembered commit, names + * another one: the visit then moves whole to it (see {@link GithubProjectReaderService.moveTo}). + */ commit: Promise; + /** How many times the visit has moved to another commit: a read that began before a move is read again after it. */ + epoch: number; mirrorUsed: boolean; - tree?: Promise; - readonly files: Map>; + tree?: IGithubRead; + readonly files: Map>; readonly json: Map>; } @@ -398,6 +428,14 @@ interface IGithubSession { * never fetches one twice. Bounded (20 MB, 20 repos, two commits per repo). * 4. A refused, failed or timed-out read from the file host is tried again from jsDelivr, same commit. * 5. When the resolve call fails, the reads degrade, never stop: see {@link GithubCommitState}. + * 6. A commit that was only remembered (an answer of the last 5 minutes, or of an earlier visit) is doubted when + * its listing is 404 or 422, or when the project file is 404 at it (whatever else was read or kept at it); any + * other 404 is an absent file. Doubting it asks GitHub again, once; if GitHub names another commit the whole visit + * moves to it (nothing read at the old one is served afterwards, and no read is delivered while the question is + * on its way), if it names the same one the commit stands. If it does not answer (refused, failed, timed out) the + * remembered commit is kept for the visit and the listing fails with the error of a refused listing, never an + * empty list; an answer that was not given is not remembered, so a later read asks again. What was remembered is + * dropped only when GitHub has answered. * Also: no credentials, no redirect followed (a renamed or moved repository is "No DataTug project here"), a * 256 KB cap per project file checked on the bytes received, at most 40 distinct files per run * ({@link GithubReadBudget}). @@ -417,20 +455,28 @@ export class GithubProjectReaderService { ); private readonly sessions = new Map(); - /** The commit each remembered commit that proved not to exist was replaced by (one replacement per commit). */ - private readonly replacements = new WeakMap< + /** + * The answer of asking again about each remembered commit that was doubted: whether the visit moved to another + * commit or the commit stands. Asked once per commit, whoever doubts it and however often, once GitHub has answered: + * an answer that was not given is not kept (see {@link askAgain}). + */ + private readonly doubts = new WeakMap< IGithubResolvedCommit, - Promise + Promise >(); + /** How many times each repository (`owner/repo`, lower case) has been forgotten: see {@link forget}. */ + private readonly generations = new Map(); /** * Forgets what this browser knows of a repository, so the next read asks GitHub again which commit it is at: the * visit's commit, files and listing, and the answer remembered for 5 minutes (at every ref). Called when a project * was just committed to the repository, which would otherwise stay "missing" until the memory expires. Files and - * listings kept by commit are not touched: a commit's content never changes. + * listings kept by commit are not touched: a commit's content never changes. A resolve that began before this call + * does not remember its answer after it. */ public async forget(org: string, repo: string): Promise { const repoKey = `${org}/${repo}`.toLowerCase(); + this.generations.set(repoKey, this.generation(repoKey) + 1); for (const key of [...this.sessions.keys()]) { if (key.startsWith(`${repoKey}@`)) { this.sessions.delete(key); @@ -439,12 +485,33 @@ export class GithubProjectReaderService { await this.store.forgetResolved(repoKey); } + private generation(repoKey: string): number { + return this.generations.get(repoKey) ?? 0; + } + // --------------------------------------------------------------------- // The commit: one per repo and ref, resolved once // --------------------------------------------------------------------- + private sessionKey(id: IGithubProjectId): string { + return `${id.org}/${id.repo}@${id.ref ?? GITHUB_DEFAULT_BRANCH_REF}`; + } + + /** + * Changes whenever the commit this visit reads a project at is replaced: the visit moved to another commit (GitHub, + * asked again, named one), or the repository was forgotten. A caller that keeps something it read (the summary of + * the project) drops it when this is not what it was when it read it. Read-only: starts nothing. Throws for an id that + * is not valid. + */ + public visitEpoch(projectId: string): string { + const id = readableId(projectId); + const repoKey = `${id.org}/${id.repo}`.toLowerCase(); + const session = this.sessions.get(this.sessionKey(id)); + return `${this.generation(repoKey)}.${session?.epoch ?? 0}`; + } + private session(id: IGithubProjectId): IGithubSession { - const key = `${id.org}/${id.repo}@${id.ref ?? GITHUB_DEFAULT_BRANCH_REF}`; + const key = this.sessionKey(id); let session = this.sessions.get(key); if (!session) { const resolve = (fresh: boolean) => this.resolveCommit(id, fresh); @@ -454,6 +521,7 @@ export class GithubProjectReaderService { ref: id.ref, resolve, commit: resolve(false), + epoch: 0, mirrorUsed: false, files: new Map(), json: new Map(), @@ -475,6 +543,8 @@ export class GithubProjectReaderService { return { state: 'given', sha: ref.toLowerCase(), remembered: false }; } const memoKey = `${org}/${repo}@${ref ?? GITHUB_DEFAULT_BRANCH_REF}`; + const repoKey = `${org}/${repo}`.toLowerCase(); + const generation = this.generation(repoKey); const memo = fresh ? undefined : await this.store.getResolved(memoKey); const now = this.now(); if (memo) { @@ -496,37 +566,86 @@ export class GithubProjectReaderService { if (out.kind === 'ok') { const sha = out.text.trim().toLowerCase(); if (COMMIT_SHA.test(sha)) { - await this.store.putResolved(memoKey, { sha, at: now }); + if (this.generation(repoKey) === generation) { + // Not when the repository was forgotten while this was asking: the answer is of the visit, not the memory. + await this.store.putResolved(memoKey, { sha, at: now }); + } return { state: 'resolved', sha, remembered: false }; } - } else if (out.kind === 'moved') { - return { state: 'moved', remembered: false }; - } else if (out.kind === 'missing') { - return { state: 'missing', remembered: false }; + } else if (out.kind === 'moved' || out.kind === 'missing') { + if (fresh || memo) { + // GitHub has answered that there is no such ref (or that the repository moved): what was remembered of it is + // of no use. Only now, and only this ref's. + await this.store.dropResolved(memoKey); + } + return { state: out.kind, remembered: false }; } return memo ? { state: 'remembered', sha: memo.sha, remembered: true } - : { state: 'unresolved', remembered: false }; + : { state: 'unresolved', remembered: false, refused: out.kind === 'refused' }; } /** - * GitHub does not know the commit a read was at, which was only remembered: the repository was rewritten, or the - * project was created after the answer was kept. The memory is dropped and the commit is resolved again, once - * (what is resolved again is never remembered, so there is no second time); every read of the session then goes on - * at the new commit, those that were waiting for GitHub's answer about the old one included (they read again). - * What was already read from the cache stays: it is the old commit's own, whole. Called once per stale commit: a - * read that finds the commit already replaced does not call it (see `replacements`). + * GitHub answered 404 or 422 for the listing of a commit that was only remembered, or 404 for its project file: the + * repository may have been rewritten since the answer was kept, or the project may have been created after it. The + * question is asked again, once per commit (what is asked is never remembered before GitHub has answered it). + * `moved` when GitHub names another commit: the visit has moved to it and the read must be redone. `stands` when + * GitHub names the same one (it exists, so the 404 is an absent file). When GitHub does not answer (the refusal, the + * failure, the timeout) the remembered commit is kept for the visit, nothing about it is remembered, and the + * question is not either: the next read that doubts the commit asks again. */ - private replaceRememberedCommit( + private doubt( session: IGithubSession, - stale: IGithubResolvedCommit, - ): Promise { - const next = this.store - .forgetResolved(`${session.org}/${session.repo}`) - .then(() => session.resolve(true)); - this.replacements.set(stale, next); - session.commit = next; - return next; + commit: IGithubResolvedCommit, + ): Promise { + let asked = this.doubts.get(commit); + if (!asked) { + asked = this.askAgain(session, commit); + this.doubts.set(commit, asked); + } + return asked; + } + + private async askAgain( + session: IGithubSession, + commit: IGithubResolvedCommit, + ): Promise { + const answer = await session.resolve(true); + if (answer.state === 'unresolved') { + this.doubts.delete(commit); // not answered: not kept, so the next read asks again + return answer.refused ? 'refused' : 'unanswered'; + } + if (answer.sha === commit.sha) { + session.commit = Promise.resolve(answer); // the same commit, now known to be what GitHub says + return 'stands'; + } + this.moveTo(session, answer); + return 'moved'; + } + + /** + * One visit never serves two commits: the session goes on at `commit`, and what was read at the old one is + * forgotten, the listing, files and decoded JSON alike. Reads still on their way are kept, and read again at the + * new commit when they come back (see `epoch`); so is the JSON decoded from a text still on its way. + */ + private moveTo(session: IGithubSession, commit: IGithubResolvedCommit): void { + session.commit = Promise.resolve(commit); + session.epoch++; + session.mirrorUsed = false; + if (session.tree?.done) { + session.tree = undefined; + } + for (const [path, read] of session.files) { + if (read.done) { + session.files.delete(path); + } + } + for (const path of session.json.keys()) { + // What was decoded is of the text it was made from: kept only while that text is still on its way. + if (session.files.get(path)?.done !== false) { + session.json.delete(path); + } + } } /** @@ -537,16 +656,23 @@ export class GithubProjectReaderService { return defer(() => { const session = this.session(readableId(projectId)); return from( - session.commit.then( - (c): IGithubReadInfo => ({ + (async (): Promise => { + let c = await session.commit; + const doubted = this.doubts.get(c); + if (doubted) { + // A question about the commit is on its way: it is not named before GitHub has answered (the wait is bounded). + await doubted; + c = await session.commit; + } + return { org: session.org, repo: session.repo, state: c.state, commit: c.sha, fromMirror: session.mirrorUsed, mayBeStale: c.state === 'remembered' || c.state === 'unresolved', - }), - ), + }; + })(), ); }); } @@ -558,30 +684,74 @@ export class GithubProjectReaderService { private getTree(projectId: string): Observable { return defer(() => { const session = this.session(readableId(projectId)); - if (!session.tree) { - const tree = this.loadTree(session); - session.tree = tree; - tree.catch(() => { - if (session.tree === tree) { + let tree = session.tree; + if (!tree) { + tree = startRead((read) => + this.loadTree(session, read), + ); + const started = tree; + session.tree = started; + started.promise.catch(() => { + if (session.tree === started) { session.tree = undefined; } }); } - return from(session.tree); + return from(tree.promise); }); } - private async loadTree( + /** + * Reads at the commit the session is at, and delivers only what is of the commit the visit stays at: a read that + * completes while GitHub is being asked whether that commit is still the one waits for the answer (bounded by the + * time the question is given), and a read the visit moved away from is read again at the new commit, whether it + * completed or failed. `read.done` is set in the same step as the answer. + */ + private async readAtCommit( session: IGithubSession, - ): Promise { - for (;;) { - const tree = await this.loadTreeAt(session, await session.commit); - if (tree !== STALE) { - return tree; + read: IGithubRead, + readAt: (commit: IGithubResolvedCommit, epoch: number) => Promise, + ): Promise { + let failures = 0; + try { + for (;;) { + const epoch = session.epoch; + const commit = await session.commit; + let value: T | typeof STALE = STALE; + let failure: { readonly error: unknown } | undefined; + try { + value = await readAt(commit, epoch); + } catch (error) { + failure = { error }; + } + await this.doubts.get(commit); + if (session.epoch === epoch) { + if (failure) { + throw failure.error; + } + if (value !== STALE) { + read.done = true; + return value; + } + } else if (failure && ++failures > MAX_REREADS_AFTER_FAILURE) { + throw failure.error; + } } + } catch (err) { + read.done = true; + throw err; } } + private loadTree( + session: IGithubSession, + read: IGithubRead, + ): Promise { + return this.readAtCommit(session, read, (commit) => + this.loadTreeAt(session, commit), + ); + } + private async loadTreeAt( session: IGithubSession, commit: IGithubResolvedCommit, @@ -620,15 +790,24 @@ export class GithubProjectReaderService { } return tree; } - case 'missing': - if (this.replacements.has(commit)) { - return STALE; // another read found the commit gone while this one was asking - } - if (commit.remembered) { - await this.replaceRememberedCommit(session, commit); + case 'missing': { + // A remembered commit that the listing does not know may be gone: ask again which commit it is. + const outcome = commit.remembered + ? await this.doubt(session, commit) + : 'stands'; + if (outcome === 'moved') { return STALE; } + // GitHub was not asked or did not answer, and the listing of the commit is not to be had: not an empty + // project, a failed read (no fallback to `HEAD`: that is not the commit the rest of the visit reads). + if (outcome === 'refused') { + throw new Error(GITHUB_RATE_LIMIT_MESSAGE); + } + if (outcome === 'unanswered') { + throw new GithubReadError('the project listing', [GITHUB_API_HOST]); + } return []; + } case 'moved': return []; case 'refused': @@ -700,34 +879,35 @@ export class GithubProjectReaderService { ): Promise { let read = session.files.get(path); if (!read) { - read = this.loadText(session, path); - session.files.set(path, read); - const memo = read; - read.catch(() => { - if (session.files.get(path) === memo) { + const started = startRead((r) => + this.loadText(session, path, r), + ); + session.files.set(path, started); + started.promise.catch(() => { + if (session.files.get(path) === started) { session.files.delete(path); } }); + read = started; } - return read; + return read.promise; } - private async loadText( + private loadText( session: IGithubSession, path: string, + read: IGithubRead, ): Promise { - for (;;) { - const text = await this.loadTextAt(session, await session.commit, path); - if (text !== STALE) { - return text; - } - } + return this.readAtCommit(session, read, (commit, epoch) => + this.loadTextAt(session, commit, path, epoch), + ); } private async loadTextAt( session: IGithubSession, commit: IGithubResolvedCommit, path: string, + epoch: number, ): Promise { if (commit.state === 'missing' || commit.state === 'moved') { return undefined; @@ -759,7 +939,10 @@ export class GithubProjectReaderService { `https://${GITHUB_MIRROR_HOST}/gh/${org}/${repo}${version ? `@${encodeURIComponent(version)}` : ''}/${path}`, { maxBytes: MAX_PROJECT_FILE_BYTES }, ); - session.mirrorUsed ||= out.kind === 'ok'; + // Not when the visit has moved on meanwhile: the sources line is of the commit the visit is at. + if (out.kind === 'ok' && session.epoch === epoch) { + session.mirrorUsed = true; + } } switch (out.kind) { case 'ok': @@ -772,25 +955,29 @@ export class GithubProjectReaderService { return out.text; case 'missing': if (out !== raw) { - // The file host would not answer and the mirror says no: the mirror lags behind GitHub (a commit pushed a - // moment ago is not there yet), so that is no word on the file. Only GitHub's own file host can say it is - // absent. + // The file host would not answer and the mirror says no. At no commit that is no word on the file (it is + // a different file, or none, as it is now). At a commit the mirror lags behind GitHub when a commit was + // pushed a moment ago, so the file may well be there: for an optional file that is an absent file for this + // visit, not kept; for the project file, which a project just created is about to be, it is a failed read + // (asking again works). + if (commit.sha && !isProjectFile(path)) { + return undefined; + } throw new GithubReadError(decodeURIComponent(path), hosts); } - if (this.replacements.has(commit)) { - return STALE; // another read found the commit gone while this one was asking - } - if (commit.remembered) { - // Every file of a commit that does not exist is "missing", the project file included: ask once more which - // commit it is, instead of taking a remembered answer for the truth (a file that really is absent costs - // this one question per visit, and is then remembered as absent). - await this.replaceRememberedCommit(session, commit); - return STALE; - } // Absent at a commit stays absent: remembered too, so a warm load asks for nothing. Not the project file: it // is the one file that a visitor is about to create, and the answer that it is not there is for this visit. - if (commitKey && !isProjectFile(path)) { - await this.store.putFile(commitKey, path, { text: null, bytes: 0 }); + if (!isProjectFile(path)) { + if (commitKey) { + await this.store.putFile(commitKey, path, { text: null, bytes: 0 }); + } + return undefined; + } + // The project file is 404 at a commit that was only remembered: the repository may have been rewritten, or + // the project created (pushed from elsewhere) after the answer was kept, whatever else was read at it. Ask + // once more which commit it is. Not answered: the commit stands for the visit, and the file is absent. + if (commit.remembered && (await this.doubt(session, commit)) === 'moved') { + return STALE; } return undefined; case 'moved': @@ -835,35 +1022,40 @@ export class GithubProjectReaderService { ): Observable { return defer(() => { const { session, path } = this.prepare(projectId, relativePath, options); - let parsed = session.json.get(path) as - | Promise - | undefined; + let parsed = session.json.get(path); if (!parsed) { - parsed = this.readText(session, path).then((text) => { - if (text === undefined) { - return undefined; - } - if (text.trim() === '') { - return null; - } - try { - return JSON.parse(text) as T; - } catch { - throw new Error(`${decodeURIComponent(path)} is not valid JSON`); - } - }); - session.json.set(path, parsed); - const memo = parsed; - parsed.catch(() => { - if (session.json.get(path) === memo) { + const started = this.decodeJson(session, path); + session.json.set(path, started); + started.catch(() => { + if (session.json.get(path) === started) { session.json.delete(path); } }); + parsed = started; } - return from(parsed); + return from(parsed as Promise); }); } + /** The decoded JSON of a file's text. */ + private async decodeJson( + session: IGithubSession, + path: string, + ): Promise { + const text = await this.readText(session, path); + if (text === undefined) { + return undefined; + } + if (text.trim() === '') { + return null; + } + try { + return JSON.parse(text); + } catch { + throw new Error(`${decodeURIComponent(path)} is not valid JSON`); + } + } + /** Same as {@link getRawJson}, but for a plain-text body (a query's `.sql`/`.dtql`/`.http` sidecar). */ public getRawText( projectId: string, diff --git a/libs/datatug/main/src/lib/services/repo/github/github-repos.service.spec.ts b/libs/datatug/main/src/lib/services/repo/github/github-repos.service.spec.ts new file mode 100644 index 00000000..ffa60c39 --- /dev/null +++ b/libs/datatug/main/src/lib/services/repo/github/github-repos.service.spec.ts @@ -0,0 +1,82 @@ +import { provideHttpClient } from '@angular/common/http'; +import { + HttpTestingController, + provideHttpClientTesting, +} from '@angular/common/http/testing'; +import { TestBed } from '@angular/core/testing'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; + +import { GithubRepoError } from './github-api'; +import { GithubReposService } from './github-repos.service'; + +describe('GithubReposService', () => { + let service: GithubReposService; + let http: HttpTestingController; + + beforeEach(() => { + TestBed.configureTestingModule({ + providers: [provideHttpClient(), provideHttpClientTesting()], + }); + service = TestBed.inject(GithubReposService); + http = TestBed.inject(HttpTestingController); + }); + + afterEach(() => http.verify()); + + it('lists the repositories the user can push to, leaving out one that has no default branch to commit to', () => { + let repos: unknown; + service.listRepos('tok').subscribe((r) => (repos = r)); + http + .expectOne((r) => r.url === 'https://api.github.com/user/repos') + .flush([ + { full_name: 'datatug/a', private: false, default_branch: 'main' }, + { full_name: 'datatug/b', private: true }, + { private: false, default_branch: 'main' }, + ]); + expect(repos).toEqual([ + { fullName: 'datatug/a', private: false, defaultBranch: 'main' }, + ]); + }); + + it('creates a repository and returns it with the default branch GitHub named', () => { + let created: unknown; + service.createRepo('tok', 'mine', true).subscribe((r) => (created = r)); + const req = http.expectOne( + (r) => + r.method === 'POST' && r.url === 'https://api.github.com/user/repos', + ); + expect(req.request.body).toEqual({ + name: 'mine', + private: true, + auto_init: true, + }); + req.flush({ full_name: 'me/mine', private: true, default_branch: 'trunk' }); + expect(created).toEqual({ + fullName: 'me/mine', + private: true, + defaultBranch: 'trunk', + }); + }); + + it.each([ + [ + { full_name: 'me/mine', private: true }, + 'GitHub created mine, but did not return the default branch of mine, so DataTug cannot tell which branch to commit to.', + ], + [ + { default_branch: 'main' }, + 'GitHub created mine, but did not return its full name', + ], + ])( + 'an incomplete answer for the repository just made is an error that says so: %j', + (wire, message) => { + let error: unknown; + service + .createRepo('tok', 'mine', true) + .subscribe({ error: (e) => (error = e) }); + http.expectOne((r) => r.method === 'POST').flush(wire); + expect(error).toBeInstanceOf(GithubRepoError); + expect((error as Error).message).toBe(message); + }, + ); +}); diff --git a/libs/datatug/main/src/lib/services/repo/github/github-repos.service.ts b/libs/datatug/main/src/lib/services/repo/github/github-repos.service.ts index 777184ec..ce129996 100644 --- a/libs/datatug/main/src/lib/services/repo/github/github-repos.service.ts +++ b/libs/datatug/main/src/lib/services/repo/github/github-repos.service.ts @@ -7,6 +7,7 @@ import { githubApiHeaders, IGithubRepo, IGithubRepoWire, + requireGithubRepo, toGithubRepo, } from './github-api'; @@ -60,15 +61,7 @@ export class GithubReposService { { headers: githubApiHeaders(token) }, ) .pipe( - map((repo) => { - const created = toGithubRepo(repo); - if (!created) { - throw new Error( - `GitHub created ${name} but did not return its full name`, - ); - } - return created; - }), + map((repo) => requireGithubRepo(repo, name, true)), ); } }