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

Public Auto Persistent Index Creation API #11757

Merged
merged 11 commits into from
Sep 5, 2023
4 changes: 4 additions & 0 deletions Firestore/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# Unreleased
- [feature] Add the option to allow the SDK to create cache indexes automatically to
improve query execution locally. (#11596)

# 10.12.0
- [feature] Implemented an optimization in the local cache synchronization logic
that reduces the number of billed document reads when documents were deleted
Expand Down
26 changes: 25 additions & 1 deletion Firestore/Example/Firestore.xcodeproj/project.pbxproj

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -1844,7 +1844,7 @@ - (void)testGetValidPersistentCacheIndexManager {
XCTAssertNotNil(db2.persistentCacheIndexManager);

// Disable persistent disk cache
FIRFirestore *db3 = [FIRFirestore firestoreForDatabase:@"PersistentCacheIndexManagerDB3"];
FIRFirestore *db3 = [FIRFirestore firestoreForDatabase:@"MemoryCacheIndexManagerDB1"];
FIRFirestoreSettings *settings3 = [db3 settings];
[settings3 setCacheSettings:[[FIRMemoryCacheSettings alloc] init]];
[db3 setSettings:settings3];
Expand Down
14 changes: 8 additions & 6 deletions Firestore/Example/Tests/Integration/API/FIRIndexingTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,17 @@
* limitations under the License.
*/

// TODO(csi): Delete this once setIndexConfigurationFromJSON and setIndexConfigurationFromStream
// are removed.
#pragma clang diagnostic ignored "-Wdeprecated-declarations"

#import <FirebaseFirestore/FirebaseFirestore.h>

#import <XCTest/XCTest.h>

#import "Firestore/Source/API/FIRFirestore+Internal.h"
#import "Firestore/Source/API/FIRPersistentCacheIndexManager+Internal.h"

#import "Firestore/Example/Tests/Util/FSTHelpers.h"
#import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"
#import "Firestore/Source/Public/FirebaseFirestore/FIRPersistentCacheIndexManager.h"

@interface FIRIndexingTests : FSTIntegrationTestCase
@end
Expand Down Expand Up @@ -194,13 +196,13 @@ - (void)testAutoIndexCreationSetSuccessfullyUsingDefault {
- (void)testAutoIndexCreationAfterFailsTermination {
[self terminateFirestore:self.db];

FSTAssertThrows([self.db.persistentCacheIndexManager enableIndexAutoCreation],
XCTAssertThrows([self.db.persistentCacheIndexManager enableIndexAutoCreation],
@"The client has already been terminated.");

FSTAssertThrows([self.db.persistentCacheIndexManager disableIndexAutoCreation],
XCTAssertThrows([self.db.persistentCacheIndexManager disableIndexAutoCreation],
@"The client has already been terminated.");

FSTAssertThrows([self.db.persistentCacheIndexManager deleteAllIndexes],
XCTAssertThrows([self.db.persistentCacheIndexManager deleteAllIndexes],
@"The client has already been terminated.");
}

Expand Down
27 changes: 27 additions & 0 deletions Firestore/Example/Tests/Util/FSTExceptionCatcher.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright 2023 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

@interface FSTExceptionCatcher : NSObject

typedef void (^ThrowingBlock)(void);

+ (BOOL)catchException:(ThrowingBlock)block error:(__autoreleasing NSError **)error;

@end

NS_ASSUME_NONNULL_END
40 changes: 40 additions & 0 deletions Firestore/Example/Tests/Util/FSTExceptionCatcher.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Copyright 2023 Google LLC
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

#import "Firestore/Example/Tests/Util/FSTExceptionCatcher.h"

@implementation FSTExceptionCatcher

+ (BOOL)catchException:(ThrowingBlock)block error:(__autoreleasing NSError **)error {
@try {
block();
return YES;
} @catch (NSException *exception) {
NSMutableDictionary *info = [NSMutableDictionary dictionary];
[info setValue:exception.name forKey:@"ExceptionName"];
[info setValue:exception.reason forKey:@"ExceptionReason"];
[info setValue:exception.callStackReturnAddresses forKey:@"ExceptionCallStackReturnAddresses"];
[info setValue:exception.callStackSymbols forKey:@"ExceptionCallStackSymbols"];
[info setValue:exception.userInfo forKey:@"ExceptionUserInfo"];

// Just using error code `FIRErrorCodeConfigFailed` for now
NSInteger FIRErrorCodeConfigFailed = -114;
*error = [[NSError alloc] initWithDomain:exception.name
code:FIRErrorCodeConfigFailed
userInfo:info];
return NO;
}
}

@end
3 changes: 0 additions & 3 deletions Firestore/Source/API/FIRFirestore+Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@ NS_ASSUME_NONNULL_BEGIN

- (const std::shared_ptr<firebase::firestore::util::AsyncQueue> &)workerQueue;

// TODO(csi): make this function public
@property(nonatomic, readonly) FIRPersistentCacheIndexManager *persistentCacheIndexManager;

@property(nonatomic, assign, readonly) std::shared_ptr<api::Firestore> wrapped;

@property(nonatomic, assign, readonly) const model::DatabaseId &databaseID;
Expand Down
8 changes: 7 additions & 1 deletion Firestore/Source/API/FIRFirestore.mm
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
* limitations under the License.
*/

// TODO(csi): Delete this once setIndexConfigurationFromJSON and setIndexConfigurationFromStream
// are removed.
#pragma clang diagnostic ignored "-Wdeprecated-declarations"

#import "FIRFirestore+Internal.h"

#include <memory>
Expand Down Expand Up @@ -535,12 +539,14 @@ @implementation FIRFirestore (Internal)
return _firestore->worker_queue();
}

- (FIRPersistentCacheIndexManager *)persistentCacheIndexManager {
- (nullable FIRPersistentCacheIndexManager *)persistentCacheIndexManager {
if (!_indexManager) {
auto index_manager = _firestore->persistent_cache_index_manager();
if (index_manager) {
_indexManager = [[FIRPersistentCacheIndexManager alloc]
initWithPersistentCacheIndexManager:index_manager];
} else {
return nil;
}
}
return _indexManager;
Expand Down
38 changes: 1 addition & 37 deletions Firestore/Source/API/FIRPersistentCacheIndexManager+Internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,51 +13,15 @@
* See the License for the specific language governing permissions and
* limitations under the License.
*/
#import "FIRPersistentCacheIndexManager.h"

#import <Foundation/Foundation.h>

#include <memory>

#include "Firestore/core/src/api/persistent_cache_index_manager.h"

NS_ASSUME_NONNULL_BEGIN

// TODO(sum/avg) move the contents of this category to
// ../Public/FirebaseFirestore/FIRPersistentCacheIndexManager.h
/**
* A PersistentCacheIndexManager which you can config persistent cache indexes used for
* local query execution.
*/
NS_SWIFT_NAME(PersistentCacheIndexManager)
@interface FIRPersistentCacheIndexManager : NSObject

/** :nodoc: */
- (instancetype)init
__attribute__((unavailable("FIRPersistentCacheIndexManager cannot be created directly.")));

/**
* Enables SDK to create persistent cache indexes automatically for local query execution when SDK
* believes cache indexes can help improves performance.
*
* This feature is disabled by default.
*/
- (void)enableIndexAutoCreation NS_SWIFT_NAME(enableIndexAutoCreation());

/**
* Stops creating persistent cache indexes automatically for local query execution. The indexes
* which have been created by calling `enableIndexAutoCreation` still take effect.
*/
- (void)disableIndexAutoCreation NS_SWIFT_NAME(disableIndexAutoCreation());

/**
* Removes all persistent cache indexes. Please note this function will also deletes indexes
* generated by [[FIRFirestore firestore] setIndexConfigurationFromJSON] and [[FIRFirestore
* firestore] setIndexConfigurationFromStream], which are deprecated.
*/
- (void)deleteAllIndexes NS_SWIFT_NAME(deleteAllIndexes());

@end

@interface FIRPersistentCacheIndexManager (/* Init */)

- (instancetype)initWithPersistentCacheIndexManager:
Expand Down
26 changes: 22 additions & 4 deletions Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
@class FIRTransaction;
@class FIRTransactionOptions;
@class FIRWriteBatch;
@class FIRPersistentCacheIndexManager;

NS_ASSUME_NONNULL_BEGIN

Expand Down Expand Up @@ -103,7 +104,16 @@ NS_SWIFT_NAME(Firestore)
#pragma mark - Configure FieldIndexes

/**
* This method is in preview. API signature and functionality are subject to change.
* A PersistentCacheIndexManager which you can config persistent cache indexes used for
* local query execution.
*/
@property(nonatomic, readonly, nullable)
FIRPersistentCacheIndexManager *persistentCacheIndexManager;

/**
* NOTE: This preview method will be deprecated in a future major release. Consider using
* `PersistentCacheIndexManager.enableIndexAutoCreation()` to let the SDK decide whether to create
* cache indexes for queries running locally.
*
* Configures indexing for local query execution. Any previous index configuration is overridden.
*
Expand All @@ -121,10 +131,15 @@ NS_SWIFT_NAME(Firestore)
*/
- (void)setIndexConfigurationFromJSON:(NSString *)json
completion:(nullable void (^)(NSError *_Nullable error))completion
NS_SWIFT_NAME(setIndexConfiguration(_:completion:));
NS_SWIFT_NAME(setIndexConfiguration(_:completion:)) DEPRECATED_MSG_ATTRIBUTE(
"Instead of creating cache indexes manually, consider using "
"`PersistentCacheIndexManager.enableIndexAutoCreation()` to let the SDK decide whether to "
"create cache indexes for queries running locally.");
Comment on lines +134 to +137
Copy link
Member

Choose a reason for hiding this comment

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

@sunmou99, it'd be a nice-to-have if API diff tool caught deprecations as well. Here, the ObjC and Swift API were both deprecated.


/**
* This method is in preview. API signature and functionality are subject to change.
* NOTE: This preview method will be deprecated in a future major release. Consider using
* `PersistentCacheIndexManager.enableIndexAutoCreation()` to let the SDK decide whether to create
* cache indexes for queries running locally.
*
* Configures indexing for local query execution. Any previous index configuration is overridden.
*
Expand All @@ -145,7 +160,10 @@ NS_SWIFT_NAME(Firestore)
*/
- (void)setIndexConfigurationFromStream:(NSInputStream *)stream
completion:(nullable void (^)(NSError *_Nullable error))completion
NS_SWIFT_NAME(setIndexConfiguration(_:completion:));
NS_SWIFT_NAME(setIndexConfiguration(_:completion:)) DEPRECATED_MSG_ATTRIBUTE(
"Instead of creating cache indexes manually, consider using "
"`PersistentCacheIndexManager.enableIndexAutoCreation()` to let the SDK decide whether to "
"create cache indexes for queries running locally.");

#pragma mark - Collections and Documents

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2023 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

#import <Foundation/Foundation.h>

NS_ASSUME_NONNULL_BEGIN

/**
* A PersistentCacheIndexManager which you can config persistent cache indexes used for
cherylEnkidu marked this conversation as resolved.
Show resolved Hide resolved
* local query execution.
*/
NS_SWIFT_NAME(PersistentCacheIndexManager)
@interface FIRPersistentCacheIndexManager : NSObject

/** :nodoc: */
- (instancetype)init
__attribute__((unavailable("FIRPersistentCacheIndexManager cannot be created directly.")));

/**
* Enables the SDK to create persistent cache indexes automatically for local query execution when
* the SDK believes cache indexes can improve performance.
*
* This feature is disabled by default.
*/
- (void)enableIndexAutoCreation NS_SWIFT_NAME(enableIndexAutoCreation());

/**
* Stops creating persistent cache indexes automatically for local query execution. The indexes
* which have been created by calling `enableIndexAutoCreation` still take effect.
*/
- (void)disableIndexAutoCreation NS_SWIFT_NAME(disableIndexAutoCreation());

/**
* Removes all persistent cache indexes. Please note this function also deletes indexes generated by
* [[FIRFirestore firestore] setIndexConfigurationFromJSON] and [[FIRFirestore firestore]
* setIndexConfigurationFromStream], which are deprecated.
*/
- (void)deleteAllIndexes NS_SWIFT_NAME(deleteAllIndexes());

@end

NS_ASSUME_NONNULL_END
1 change: 1 addition & 0 deletions Firestore/Swift/Tests/BridgingHeader.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#define FIRESTORE_SWIFT_TESTS_BRIDGINGHEADER_H_

#import "Firestore/Example/Tests/API/FSTAPIHelpers.h"
#import "Firestore/Example/Tests/Util/FSTExceptionCatcher.h"
#import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"

#endif /* FIRESTORE_SWIFT_TESTS_BRIDGINGHEADER_H_ */
28 changes: 28 additions & 0 deletions Firestore/Swift/Tests/Integration/AsyncAwaitIntegrationTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -93,5 +93,33 @@ let emptyBundle = """
XCTAssertEqual(FIRQuerySnapshotGetIDs(snapshot),
["doc1", "doc2", "doc4", "doc5"])
}

func testAutoIndexCreationAfterFailsTermination() async throws {
try await db.terminate()

let enableIndexAutoCreation = {
try FSTExceptionCatcher.catchException {
self.db.persistentCacheIndexManager?.enableIndexAutoCreation()
}
}
XCTAssertThrowsError(try enableIndexAutoCreation(), "The client has already been terminated.")

let disableIndexAutoCreation = {
try FSTExceptionCatcher.catchException {
self.db.persistentCacheIndexManager?.disableIndexAutoCreation()
}
}
XCTAssertThrowsError(
try disableIndexAutoCreation(),
"The client has already been terminated."
)

let deleteAllIndexes = {
try FSTExceptionCatcher.catchException {
self.db.persistentCacheIndexManager?.deleteAllIndexes()
}
}
XCTAssertThrowsError(try deleteAllIndexes(), "The client has already been terminated.")
}
}
#endif
Loading
Loading