Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Firestore emulator ClearData fails when deleting more than 500 documents #1468

Closed
mad-it opened this issue Jul 4, 2019 · 8 comments · Fixed by #1484
Closed

Firestore emulator ClearData fails when deleting more than 500 documents #1468

mad-it opened this issue Jul 4, 2019 · 8 comments · Fixed by #1484

Comments

@mad-it
Copy link

mad-it commented Jul 4, 2019

[REQUIRED] Environment info

firebase-tools:
Using NPM:

"firebase-admin": "^8.2.0",
"firebase-functions": "^3.0.2",
"firebase-tools": "^7.0.2",

Platform:
Ubuntu 19.04

[REQUIRED] Test case

Steps to reproduce described below. The behaviour of clear data of firestore emulator changed from version 1.5.0 to 1.6.0. We have written a nice wrapper around firebase emulator to support transactions because support did not exist a long time ago in the emulator. This wrapper also clears up the data in emulator after each test.
Our wrapper uses the GRPC proto to call firestore emulator. We are using /google.firestore.emulator.v1.FirestoreEmulator/ClearData to clear the data after each test.

[REQUIRED] Steps to reproduce

import { firestore } from 'firebase-admin';

describe('firestore cleanup failure', (): void => {
  it('creates 500 documents', async (): Promise<void> => {
    const batch = firestore().batch();

    const ref = firestore().collection('docs');

    for (let index = 0; index < 500; index++) {
      batch.create(ref.doc(), {});
    }
    await batch.commit();

    const docsSnapshot = await firestore()
      .collection('docs')
      .get();

    expect(docsSnapshot.size).toStrictEqual(500);
  });

  it('should be a clean start', async (): Promise<void> => {
    const docsSnapshot = await firestore()
      .collection('docs')
      .get();

    expect(docsSnapshot.size).toStrictEqual(0);
  });
});

output:


> jest "src/test.spec.ts"

 FAIL  src/test.spec.ts (24.79s)
  firestore cleanup failure
    ✓ creates 500 documents (7554ms)
    ✕ should be a clean start (5051ms)

  ● firestore cleanup failure › should be a clean start

    expect(received).toStrictEqual(expected) // deep equality

    Expected: 0
    Received: 500

      24 |       .get();
      25 | 
    > 26 |     expect(docsSnapshot.size).toStrictEqual(0);
         |                               ^
      27 |   });
      28 | });
      29 | 

      at Object.it (src/test.spec.ts:26:31)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 passed, 2 total
Snapshots:   0 total
Time:        24.848s

[REQUIRED] Expected behavior

The second test should start with a clean slate.

[REQUIRED] Actual behavior

The second test does not have a clean start due to the data still being present.

Extra information

const PROTO_ROOT = resolve(__dirname, 'protocols');
const PROTO_FILE = 'google/firestore/emulator/v1/firestore_emulator.proto';

const makeEmulatorClient = once<Promise<Client & any>>(
  async (): Promise<Client> => {
    /* eslint-disable unicorn/prevent-abbreviations */
    const packageDefinition = await load(PROTO_FILE, {
      includeDirs: [PROTO_ROOT],
    });
    /* eslint-enable unicorn/prevent-abbreviations */

    const protoDescriptor = loadPackageDefinition(packageDefinition);
    const service = path(
      ['google', 'firestore', 'emulator', 'v1'],
      protoDescriptor,
    ) as GrpcObject;
    const FirestoreEmulator = service.FirestoreEmulator as typeof Client;

    const address = `${FIRESTORE_SERVICE_HOSTNAME}:${FIRESTORE_PORT}`;
    const sslCredentials = credentials.createInsecure();

    const client = new FirestoreEmulator(address, sslCredentials);

    return client;
  },
);

const clearFirestoreData = async (projectId: string): Promise<void> => {
  const client = await makeEmulatorClient();
  const database = `projects/${projectId}/databases/(default)`;

  /*
   * Do not convert this implementation to use `promisify` as it seemingly
   * breaks the functionality.
   */
  return new Promise((resolvePromise): void => {
    client.clearData({ database }, resolvePromise);
  });
};
@google-oss-bot
Copy link
Contributor

This issue does not have all the information required by the template. Looks like you forgot to fill out some sections. Please update the issue with more information.

@ryanpbrewster
Copy link
Contributor

Hrm. The Firestore emulator quickstart relies on the same ClearData RPC that your wrapper client is invoking, and it is still working with v1.6.0.

I don't think we have enough information here to reproduce the issue. Can you provide a short, self-contained snippet that doesn't do what you expect it to? Something like

  • create a document
  • call clearData
  • assert that the document is no longer there
  • assertion fails

@mad-it
Copy link
Author

mad-it commented Jul 4, 2019

I have created a self contained repo that should represent the issue at hand. Should work with emulator 1.5.0 but not with 1.6.0

https://github.com/mad-it/firestore-1468

@ryanpbrewster
Copy link
Contributor

I cannot get the tests in that repo to pass with any version of the emulator.

> java -jar ~/.cache/firebase/emulators/cloud-firestore-emulator-v1.5.0.jar
> npm test
    expect(received).toStrictEqual(expected) // deep equality

    Expected: 0
    Received: 500
> java -jar ~/.cache/firebase/emulators/cloud-firestore-emulator-v1.6.0.jar
> npm test
    expect(received).toStrictEqual(expected) // deep equality

    Expected: 0
    Received: 500

I've also tried it with v1.4.5 with no success.

@ryanpbrewster
Copy link
Contributor

Could you try running this test:

const firebase = require("@firebase/testing");
const expect = require("chai").expect;

const projectId = "firestore-emulator-example";

it("clearData should clear data", async () => {
  const db = firebase.initializeTestApp({ projectId }).firestore();

  const doc = db.collection("users").doc("alice");

  await doc.set({ hello: "world" });

  const read1 = await doc.get();
  expect(read1.exists).to.eql(true);
  expect(read1.data()).to.eql({ hello: "world" });

  await firebase.clearFirestoreData({ projectId });

  const read2 = await doc.get();
  expect(read2.exists).to.eql(false);
  expect(read2.data()).to.eql(undefined);
});

This examples passes for me on both v1.5.0 and v1.6.0

@mad-it
Copy link
Author

mad-it commented Jul 5, 2019

Updated the repo with actual clear reproducability. You will see it fail on 1.6.0 and succeed on 1.5.0.

https://github.com/mad-it/firestore-1468

> java -jar cloud-firestore-emulator-v1.5.0.jar
> npx ts-node ./test.ts 
501
0
> java -jar cloud-firestore-emulator-v1.6.0.jar
> npx ts-node ./test.ts 
501
501

The issue is:

Jul 05, 2019 11:47:04 AM com.google.cloud.datastore.emulator.impl.util.WrappedStreamObserver onError
INFO: operation failed: cannot write more than 500 entities in a single call

@ryanpbrewster
Copy link
Contributor

Ah, good catch. This is probably a regression due to a related bugfix. Should be a simple fix on our end.

@ryanpbrewster ryanpbrewster changed the title Firestore emulator clear data not working properly. Firestore emulator ClearData fails when deleting more than 500 documents Jul 5, 2019
@ryanpbrewster
Copy link
Contributor

Fix merged, will go out with the next release of firebase-tools. Thanks for reporting :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants