diff --git a/src/database/listRemote.ts b/src/database/listRemote.ts new file mode 100644 index 00000000000..86f4a3db243 --- /dev/null +++ b/src/database/listRemote.ts @@ -0,0 +1,84 @@ +import * as request from "request"; +import { Response } from "request"; +import * as responseToError from "../responseToError"; +import * as utils from "../utils"; +import * as FirebaseError from "../error"; +import * as logger from "../logger"; +import * as api from "../api"; + +export interface ListRemote { + /** + * Call the shallow get API with limitToFirst=numSubPath. + * @param path the path to list + * @param numSubPath the number of subPaths to fetch. + * @param startAfter omit list entries comparing lower than `startAfter` + * @param timeout milliseconds after which to timeout the request + * @return the list of sub pathes found. + */ + listPath( + path: string, + numSubPath: number, + startAfter?: string, + timeout?: number + ): Promise; +} + +export class RTDBListRemote implements ListRemote { + constructor(private instance: string) {} + + async listPath( + path: string, + numSubPath: number, + startAfter?: string, + timeout?: number + ): Promise { + const url = `${utils.addSubdomain(api.realtimeOrigin, this.instance)}${path}.json`; + + const params: any = { + shallow: true, + limitToFirst: numSubPath, + }; + if (startAfter) { + params.startAfter = startAfter; + } + if (timeout) { + params.timeout = `${timeout}ms`; + } + + const t0 = Date.now(); + const reqOptionsWithToken = await api.addRequestHeaders({ url }); + reqOptionsWithToken.qs = params; + const paths = await new Promise((resolve, reject) => { + request.get(reqOptionsWithToken, (err: Error, res: Response, body: any) => { + if (err) { + return reject( + new FirebaseError("Unexpected error while listing subtrees", { + exit: 2, + original: err, + }) + ); + } else if (res.statusCode >= 400) { + return reject(responseToError(res, body)); + } + let data; + try { + data = JSON.parse(body); + } catch (e) { + return reject( + new FirebaseError("Malformed JSON response in shallow get ", { + exit: 2, + original: e, + }) + ); + } + if (data) { + return resolve(Object.keys(data)); + } + return resolve([]); + }); + }); + const dt = Date.now() - t0; + logger.debug(`[database] sucessfully fetched ${paths.length} path at ${path} ${dt}`); + return paths; + } +} diff --git a/src/database/remove.ts b/src/database/remove.ts index 131c4072ad0..aa5d623695f 100644 --- a/src/database/remove.ts +++ b/src/database/remove.ts @@ -1,6 +1,7 @@ import * as pathLib from "path"; import { RemoveRemote, RTDBRemoveRemote } from "./removeRemote"; +import { ListRemote, RTDBListRemote } from "./listRemote"; import { Stack } from "../throttler/stack"; function chunkList(ls: T[], chunkSize: number): T[][] { @@ -18,6 +19,7 @@ const MAX_LIST_NUM_SUB_PATH = 204800; export default class DatabaseRemove { path: string; remote: RemoveRemote; + listRemote: ListRemote; private deleteJobStack: Stack<() => Promise, boolean>; private listStack: Stack<() => Promise, string[]>; @@ -36,6 +38,7 @@ export default class DatabaseRemove { concurrency: 1, retries: 3, }); + this.listRemote = new RTDBListRemote(instance); this.listStack = new Stack({ name: "list stack", concurrency: 1, @@ -68,7 +71,7 @@ export default class DatabaseRemove { let batchSize = INITIAL_DELETE_BATCH_SIZE; while (true) { const subPathList = await this.listStack.run(() => - this.remote.listPath(path, listNumSubPath) + this.listRemote.listPath(path, listNumSubPath) ); if (subPathList.length === 0) { return Promise.resolve(false); diff --git a/src/database/removeRemote.ts b/src/database/removeRemote.ts index fe5d4e8f7fe..bbeb23d0923 100644 --- a/src/database/removeRemote.ts +++ b/src/database/removeRemote.ts @@ -19,14 +19,6 @@ export interface RemoveRemote { * @return false if the deleteion failed because the the total size of subpaths exceeds the writeSizeLimit. */ deleteSubPath(path: string, subPaths: string[]): Promise; - - /** - * Call the shallow get API with limitToFirst=numSubPath. - * @param path the path to list - * @param numSubPath the number of subPaths to fetch. - * @return the list of sub pathes found. - */ - listPath(path: string, numSubPath: number): Promise; } export class RTDBRemoveRemote implements RemoveRemote { @@ -48,55 +40,6 @@ export class RTDBRemoveRemote implements RemoveRemote { return this.patch(path, body, `${subPaths.length} subpaths`); } - listPath(path: string, numSubPath: number): Promise { - const url = - utils.addSubdomain(api.realtimeOrigin, this.instance) + - path + - `.json?shallow=true&limitToFirst=${numSubPath}`; - const t0 = Date.now(); - return api - .addRequestHeaders({ - url, - }) - .then((reqOptionsWithToken) => { - return new Promise((resolve, reject) => { - request.get(reqOptionsWithToken, (err: Error, res: Response, body: any) => { - if (err) { - return reject( - new FirebaseError("Unexpected error while listing subtrees", { - exit: 2, - original: err, - }) - ); - } else if (res.statusCode >= 400) { - return reject(responseToError(res, body)); - } - let data = {}; - try { - data = JSON.parse(body); - } catch (e) { - return reject( - new FirebaseError("Malformed JSON response in shallow get ", { - exit: 2, - original: e, - }) - ); - } - if (data) { - const keyList = Object.keys(data); - return resolve(keyList); - } - resolve([]); - }); - }); - }) - .then((paths: string[]) => { - const dt = Date.now() - t0; - logger.debug(`[database] Sucessfully fetched ${paths.length} path at ${path} ${dt}`); - return paths; - }); - } - private patch(path: string, body: any, note: string): Promise { const t0 = Date.now(); return new Promise((resolve, reject) => { diff --git a/src/test/database/fakeListRemote.spec.ts b/src/test/database/fakeListRemote.spec.ts new file mode 100644 index 00000000000..4905b5b4161 --- /dev/null +++ b/src/test/database/fakeListRemote.spec.ts @@ -0,0 +1,101 @@ +import * as pathLib from "path"; +import * as chai from "chai"; + +import { ListRemote } from "../../database/listRemote"; + +const expect = chai.expect; + +/** + * `FakeListRemote` is a test fixture for verifying logic lives in the + * `DatabaseRemove` class. It is essentially a mock for the Realtime Database + * that accepts a JSON tree to serve upon construction. + */ +export class FakeListRemote implements ListRemote { + data: any; + delay: number; + + /** + * @param data the fake database structure. Each leaf is an integer + * representing the subtree's size. + */ + constructor(data: any) { + this.data = data; + this.delay = 0; + } + + async listPath( + path: string, + numChildren: number, + startAfter?: string, + timeout?: number + ): Promise { + if (timeout === 0) { + throw new Error("timeout"); + } + const d = this.dataAtPath(path); + if (d) { + let keys = Object.keys(d); + /* + * We mirror a critical implementation detail of here. Namely, the + * `startAfter` option (if it exists) is applied to the resulting key set + * before the `limitToFirst` option. + */ + if (startAfter) { + keys = keys.filter((key) => key > startAfter); + } + keys = keys.slice(0, numChildren); + return keys; + } + return []; + } + + private size(data: any): number { + if (typeof data === "number") { + return data; + } + let size = 0; + for (const key of Object.keys(data)) { + size += this.size(data[key]); + } + return size; + } + + private dataAtPath(path: string): any { + const splitedPath = path.slice(1).split("/"); + let d = this.data; + for (const p of splitedPath) { + if (d && p !== "") { + if (typeof d === "number") { + d = null; + } else { + d = d[p]; + } + } + } + return d; + } +} + +describe("FakeListRemote", () => { + it("should return limit the number of subpaths returned", async () => { + const fakeDb = new FakeListRemote({ 1: 1, 2: 2, 3: 3, 4: 4 }); + await expect(fakeDb.listPath("/", 4)).to.eventually.eql(["1", "2", "3", "4"]); + await expect(fakeDb.listPath("/", 3)).to.eventually.eql(["1", "2", "3"]); + await expect(fakeDb.listPath("/", 2)).to.eventually.eql(["1", "2"]); + await expect(fakeDb.listPath("/", 1)).to.eventually.eql(["1"]); + await expect(fakeDb.listPath("/", 4, "1")).to.eventually.eql(["2", "3", "4"]); + await expect(fakeDb.listPath("/", 4, "2")).to.eventually.eql(["3", "4"]); + await expect(fakeDb.listPath("/", 4, "3")).to.eventually.eql(["4"]); + await expect(fakeDb.listPath("/", 4, "4")).to.eventually.eql([]); + await expect(fakeDb.listPath("/", 3, "1")).to.eventually.eql(["2", "3", "4"]); + await expect(fakeDb.listPath("/", 3, "2")).to.eventually.eql(["3", "4"]); + await expect(fakeDb.listPath("/", 3, "3")).to.eventually.eql(["4"]); + await expect(fakeDb.listPath("/", 3, "3")).to.eventually.eql(["4"]); + await expect(fakeDb.listPath("/", 3, "4")).to.eventually.eql([]); + await expect(fakeDb.listPath("/", 1, "1")).to.eventually.eql(["2"]); + await expect(fakeDb.listPath("/", 1, "2")).to.eventually.eql(["3"]); + await expect(fakeDb.listPath("/", 1, "3")).to.eventually.eql(["4"]); + await expect(fakeDb.listPath("/", 1, "4")).to.eventually.eql([]); + await expect(fakeDb.listPath("/", 1, "1", 0)).to.be.rejected; + }); +}); diff --git a/src/test/database/fakeRemoveRemote.spec.ts b/src/test/database/fakeRemoveRemote.spec.ts index f5fa78dab1a..965dcef55a5 100644 --- a/src/test/database/fakeRemoveRemote.spec.ts +++ b/src/test/database/fakeRemoveRemote.spec.ts @@ -19,15 +19,6 @@ export class FakeRemoveRemote implements RemoveRemote { this.largeThreshold = largeThreshold; } - listPath(path: string, numChildren: number): Promise { - const d = this._dataAtpath(path); - if (d) { - const keys = Object.keys(d); - return Promise.resolve(keys.slice(0, numChildren)); - } - return Promise.resolve([]); - } - deletePath(path: string): Promise { const d = this._dataAtpath(path); const size = this._size(d); @@ -94,14 +85,6 @@ export class FakeRemoveRemote implements RemoveRemote { } describe("FakeRemoveRemote", () => { - it("should return limit the number of subpaths returned", async () => { - const fakeDb = new FakeRemoveRemote({ 1: 1, 2: 2, 3: 3, 4: 4 }); - await expect(fakeDb.listPath("/", 4)).to.eventually.eql(["1", "2", "3", "4"]); - await expect(fakeDb.listPath("/", 3)).to.eventually.eql(["1", "2", "3"]); - await expect(fakeDb.listPath("/", 2)).to.eventually.eql(["1", "2"]); - await expect(fakeDb.listPath("/", 1)).to.eventually.eql(["1"]); - }); - it("should failed to delete large path /", async () => { const data = { 1: 11 }; const fakeDb = new FakeRemoveRemote(data); diff --git a/src/test/database/listRemote.spec.ts b/src/test/database/listRemote.spec.ts new file mode 100644 index 00000000000..0def0b7253c --- /dev/null +++ b/src/test/database/listRemote.spec.ts @@ -0,0 +1,29 @@ +import { expect } from "chai"; +import * as nock from "nock"; + +import * as utils from "../../utils"; +import * as api from "../../api"; +import * as helpers from "../helpers"; +import { RTDBListRemote } from "../../database/listRemote"; + +describe("ListRemote", () => { + const instance = "fake-db"; + const remote = new RTDBListRemote(instance); + const serverUrl = utils.addSubdomain(api.realtimeOrigin, instance); + + afterEach(() => { + nock.cleanAll(); + }); + + it("should return subpaths from shallow get request", async () => { + nock(serverUrl) + .get("/.json") + .query({ shallow: true, limitToFirst: "1234" }) + .reply(200, { + a: true, + x: true, + f: true, + }); + await expect(remote.listPath("/", 1234)).to.eventually.eql(["a", "x", "f"]); + }); +}); diff --git a/src/test/database/remove.spec.ts b/src/test/database/remove.spec.ts index bb2a72337e1..51f737a8544 100644 --- a/src/test/database/remove.spec.ts +++ b/src/test/database/remove.spec.ts @@ -3,6 +3,7 @@ import { expect } from "chai"; import DatabaseRemove from "../../database/remove"; import { RemoveRemote } from "../../database/removeRemote"; import { FakeRemoveRemote } from "./fakeRemoveRemote.spec"; +import { FakeListRemote } from "./fakeListRemote.spec"; describe("DatabaseRemove", () => { it("should remove tiny tree", async () => { @@ -14,7 +15,7 @@ describe("DatabaseRemove", () => { }); it("should remove subtree at /a/b/c", async () => { - const fakeDb = new FakeRemoveRemote({ + const data = { a: { b: { x: { y: 1 } }, c: { x: 4, y: 8 }, @@ -23,9 +24,14 @@ describe("DatabaseRemove", () => { d: { e: 3, }, - }); + }; + + const fakeList = new FakeListRemote(data); + const fakeDb = new FakeRemoveRemote(data); + const removeOps = new DatabaseRemove("test-sub-path", "/a"); removeOps.remote = fakeDb; + removeOps.listRemote = fakeList; await removeOps.execute(); expect(fakeDb.data).to.eql({ d: { @@ -48,17 +54,23 @@ describe("DatabaseRemove", () => { function databaseRemoveTestSuit(threshold: number): void { describe(`DatabaseRemove when largeThreshold=${threshold}`, () => { it("should remove nested tree", async () => { - const fakeDb = new FakeRemoveRemote(buildData(3, 5), threshold); + const data = buildData(3, 5); + const fakeDb = new FakeRemoveRemote(data, threshold); + const fakeLister = new FakeListRemote(data); const removeOps = new DatabaseRemove("test-nested-tree", "/"); removeOps.remote = fakeDb; + removeOps.listRemote = fakeLister; await removeOps.execute(); expect(fakeDb.data).to.eql(null); }); it("should remove flat tree when threshold=${threshold}", async () => { - const fakeDb = new FakeRemoveRemote(buildData(1232, 1), threshold); + const data = buildData(1232, 1); + const fakeDb = new FakeRemoveRemote(data, threshold); + const fakeList = new FakeListRemote(data); const removeOps = new DatabaseRemove("test-remover", "/"); removeOps.remote = fakeDb; + removeOps.listRemote = fakeList; await removeOps.execute(); expect(fakeDb.data).to.eql(null); }); diff --git a/src/test/database/removeRemote.spec.ts b/src/test/database/removeRemote.spec.ts index 7239adefa0d..46802939765 100644 --- a/src/test/database/removeRemote.spec.ts +++ b/src/test/database/removeRemote.spec.ts @@ -6,10 +6,12 @@ import * as api from "../../api"; import * as helpers from "../helpers"; import { RTDBRemoveRemote } from "../../database/removeRemote"; +import { RTDBListRemote } from "../../database/listRemote"; describe("RemoveRemote", () => { const instance = "fake-db"; const remote = new RTDBRemoveRemote(instance); + const listRemote = new RTDBListRemote(instance); const serverUrl = utils.addSubdomain(api.realtimeOrigin, instance); let sandbox: sinon.SinonSandbox; @@ -23,18 +25,6 @@ describe("RemoveRemote", () => { nock.cleanAll(); }); - it("should return subpaths from shallow get request", () => { - nock(serverUrl) - .get("/.json") - .query({ shallow: true, limitToFirst: "1234" }) - .reply(200, { - a: true, - x: true, - f: true, - }); - return expect(remote.listPath("/", 1234)).to.eventually.eql(["a", "x", "f"]); - }); - it("should return true when patch is small", () => { nock(serverUrl) .patch("/a/b.json")