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.

Sign up for GitHub

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

Jump to bottom

feat: add recursiveDelete() to Firestore #622

Merged
merged 10 commits into from May 24, 2021
Merged

feat: add recursiveDelete() to Firestore #622

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
246ce86
feat: add recursiveDelete() to Firestore
May 5, 2021
575d9ec
Apply suggestions from code review thanks sebastian
May 7, 2021
bfe333b
resolve comments
May 10, 2021
8b7d340
fix test
May 11, 2021
7862601
fix clirr
May 11, 2021
eb0ecd7
Merge branch 'master' into bc/rc-base
May 18, 2021
25f4eed
resolve comments + refactor constructor
May 18, 2021
9b38613
lint
May 18, 2021
61989c7
simplify overloads
May 20, 2021
47876e8
add comments, fix preconditions check
May 24, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions google-cloud-firestore/src/main/java/com/google/cloud/firestore/BulkWriter.java
View file
Open in desktop
Expand Up @@ -805,6 +805,16 @@ public void close() throws InterruptedException, ExecutionException {
flushFuture.get();
}

/**
* Used for verifying that the BulkWriter instance isn't closed when calling from outside this
* class.
*/
void verifyNotClosed() {
synchronized (lock) {
verifyNotClosedLocked();
}
}

void verifyNotClosedLocked() {
if (this.closed) {
throw new IllegalStateException("BulkWriter has already been closed.");
Expand Down
6 changes: 2 additions & 4 deletions google-cloud-firestore/src/main/java/com/google/cloud/firestore/FirestoreImpl.java
View file
Open in desktop
Expand Up @@ -149,11 +149,9 @@ public ApiFuture<Void> recursiveDelete(DocumentReference reference, BulkWriter b
*/
@Nonnull
@VisibleForTesting
public ApiFuture<Void> recursiveDelete(
ApiFuture<Void> recursiveDelete(
CollectionReference reference, BulkWriter bulkWriter, int maxLimit, int minLimit) {
RecursiveDelete deleter = new RecursiveDelete(this, bulkWriter, reference);
deleter.setMaxPendingOps(maxLimit);
deleter.setMinPendingOps(minLimit);
RecursiveDelete deleter = new RecursiveDelete(this, bulkWriter, reference, maxLimit, minLimit);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code flow would be a bit cleaner if you called this method from all overloads. This will ensue that all calls go through the same callsite. If you take parentPath and collectionId as input here then you will also remove some duplication in RecursiveDelete (as you only need one constructor).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done. RecursiveDelete only has one constructor now.

return deleter.run();
}

Expand Down
4 changes: 2 additions & 2 deletions google-cloud-firestore/src/main/java/com/google/cloud/firestore/Query.java
View file
Open in desktop
Expand Up @@ -257,7 +257,7 @@ abstract static class QueryOptions {

// Whether to select all documents under `parentPath`. By default, only
// collections that match `collectionId` are selected.
abstract boolean getKindless();
abstract boolean isKindless();

// Whether to require consistent documents when restarting the query. By
// default, restarting the query uses the readTime offset of the original
Expand Down Expand Up @@ -1254,7 +1254,7 @@ private StructuredQuery.Builder buildWithoutClientTranslation() {
CollectionSelector.Builder collectionSelector = CollectionSelector.newBuilder();

// Kindless queries select all descendant documents, so we don't add the collectionId field.
if (!options.getKindless()) {
if (!options.isKindless()) {
collectionSelector.setCollectionId(options.getCollectionId());
}

Expand Down