Skip to content

Commit 12968c3

Browse files
ehsannasehsannas
authored
Remove STLPort support from public API. (#510)
* Remove STLPort support from public API. * Don't use EventListener in integration tests.
1 parent 0ff0c6d commit 12968c3

33 files changed

+33
-403
lines changed

‎app/CMakeLists.txt

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -469,7 +469,6 @@ if (IOS)
469469
${FIREBASE_SOURCE_DIR}/firestore/src/include/firebase/firestore/document_change.h
470470
${FIREBASE_SOURCE_DIR}/firestore/src/include/firebase/firestore/document_reference.h
471471
${FIREBASE_SOURCE_DIR}/firestore/src/include/firebase/firestore/document_snapshot.h
472-
${FIREBASE_SOURCE_DIR}/firestore/src/include/firebase/firestore/event_listener.h
473472
${FIREBASE_SOURCE_DIR}/firestore/src/include/firebase/firestore/field_path.h
474473
${FIREBASE_SOURCE_DIR}/firestore/src/include/firebase/firestore/field_value.h
475474
${FIREBASE_SOURCE_DIR}/firestore/src/include/firebase/firestore/listener_registration.h

‎firestore/integration_test/src/integration_test.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -448,9 +448,6 @@ TEST_F(FirebaseFirestoreBasicTest, TestSetDelete) {
448448
}
449449

450450
TEST_F(FirebaseFirestoreBasicTest, TestDocumentListener) {
451-
SKIP_TEST_IF_USING_STLPORT; // STLPort uses EventListener<T> rather than
452-
// std::function.
453-
#if !defined(STLPORT)
454451
SignIn();
455452

456453
firebase::firestore::DocumentReference document = Doc();
@@ -487,7 +484,6 @@ TEST_F(FirebaseFirestoreBasicTest, TestDocumentListener) {
487484
{"val", firebase::firestore::FieldValue::String("start")}},
488485
firebase::firestore::MapFieldValue{
489486
{"val", firebase::firestore::FieldValue::String("update")}}));
490-
#endif // !defined(STLPORT)
491487
}
492488

493489
TEST_F(FirebaseFirestoreBasicTest, TestBatchWrite) {

‎firestore/integration_test_internal/src/cleanup_test.cc

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,8 @@ void ExpectAllMethodsAreNoOps(DocumentReference* ptr) {
101101

102102
EXPECT_TRUE(ptr->Delete() == FailedFuture<void>());
103103

104-
#if defined(FIREBASE_USE_STD_FUNCTION)
105104
ptr->AddSnapshotListener(
106105
[](const DocumentSnapshot&, Error, const std::string&) {});
107-
#else
108-
ptr->AddSnapshotListener(nullptr);
109-
#endif
110106
}
111107

112108
void ExpectAllMethodsAreNoOps(DocumentSnapshot* ptr) {
@@ -217,12 +213,8 @@ void ExpectAllMethodsAreNoOps(Query* ptr) {
217213

218214
EXPECT_TRUE(ptr->Get() == FailedFuture<QuerySnapshot>());
219215

220-
#if defined(FIREBASE_USE_STD_FUNCTION)
221216
ptr->AddSnapshotListener(
222217
[](const QuerySnapshot&, Error, const std::string&) {});
223-
#else
224-
ptr->AddSnapshotListener(nullptr);
225-
#endif
226218
}
227219

228220
void ExpectAllMethodsAreNoOps(QuerySnapshot* ptr) {
@@ -360,7 +352,6 @@ TEST_F(CleanupTest, FieldValueIsBlankAfterCleanup) {
360352
// after cleanup. Thus, there is no case where a user could be accessing
361353
// a "blank" Firestore instance.
362354

363-
#if defined(FIREBASE_USE_STD_FUNCTION)
364355
TEST_F(CleanupTest, ListenerRegistrationIsBlankAfterCleanup) {
365356
{
366357
ListenerRegistration default_constructed;
@@ -375,7 +366,6 @@ TEST_F(CleanupTest, ListenerRegistrationIsBlankAfterCleanup) {
375366
SCOPED_TRACE("ListenerRegistration.AfterCleanup");
376367
ExpectAllMethodsAreNoOps(&reg);
377368
}
378-
#endif
379369

380370
// Note: `Query` cleanup is tested as part of `CollectionReference` cleanup
381371
// (`CollectionReference` is derived from `Query`).

‎firestore/integration_test_internal/src/firestore_integration_test.h

Lines changed: 5 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -57,15 +57,15 @@ class EventAccumulator;
5757
// An EventListener class for writing tests. This listener counts the number of
5858
// events as well as keeps track of the last result.
5959
template <typename T>
60-
class TestEventListener : public EventListener<T> {
60+
class TestEventListener {
6161
public:
6262
explicit TestEventListener(std::string name) : name_(std::move(name)) {}
6363

64-
~TestEventListener() override {}
64+
virtual ~TestEventListener() {}
6565

66-
void OnEvent(const T& value,
67-
Error error_code,
68-
const std::string& error_message) override {
66+
virtual void OnEvent(const T& value,
67+
Error error_code,
68+
const std::string& error_message) {
6969
if (print_debug_info_) {
7070
std::cout << "TestEventListener got: ";
7171
if (error_code == Error::kErrorOk) {
@@ -110,20 +110,14 @@ class TestEventListener : public EventListener<T> {
110110
return last_results_[last_results_.size() - 1 - i];
111111
}
112112

113-
// Hides the STLPort-related quirk that `AddSnapshotListener` has different
114-
// signatures depending on whether `std::function` is available.
115113
template <typename U>
116114
ListenerRegistration AttachTo(
117115
U* ref, MetadataChanges metadata_changes = MetadataChanges::kExclude) {
118-
#if defined(FIREBASE_USE_STD_FUNCTION)
119116
return ref->AddSnapshotListener(
120117
metadata_changes, [this](const T& result, Error error_code,
121118
const std::string& error_message) {
122119
OnEvent(result, error_code, error_message);
123120
});
124-
#else
125-
return ref->AddSnapshotListener(metadata_changes, this);
126-
#endif
127121
}
128122

129123
std::string first_error_message() {

‎firestore/integration_test_internal/src/firestore_test.cc

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -671,26 +671,10 @@ TEST_F(FirestoreIntegrationTest,
671671
EXPECT_EQ(1, test_data.GetEventCount());
672672
test_data.ClearEvents();
673673

674-
#if defined(FIREBASE_USE_STD_FUNCTION)
675674
ListenerRegistration sync_registration =
676675
TestFirestore()->AddSnapshotsInSyncListener(
677676
[&test_data] { test_data.AddEvent("snapshots-in-sync"); });
678677

679-
#else
680-
class SyncEventListener : public EventListener<void> {
681-
public:
682-
explicit SyncEventListener(TestData& test_data) : test_data_(test_data) {}
683-
684-
void OnEvent(Error) override { test_data_.AddEvent("snapshots-in-sync"); }
685-
686-
private:
687-
TestData& test_data_;
688-
};
689-
SyncEventListener sync_listener{test_data};
690-
ListenerRegistration sync_registration =
691-
TestFirestore()->AddSnapshotsInSyncListener(sync_listener);
692-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
693-
694678
Await(document.Set(MapFieldValue{{"foo", FieldValue::Double(3.0)}}));
695679
// Wait for the snapshots-in-sync listener to fire afterwards.
696680
test_data.WaitForEventCount("snapshots-in-sync", 2);
@@ -737,7 +721,6 @@ TEST_F(FirestoreIntegrationTest, TestQueriesAreValidatedOnClient) {
737721
// The test harness will generate Java JUnit test regardless whether this is
738722
// inside a #if or not. So we move #if inside instead of enclose the whole case.
739723
TEST_F(FirestoreIntegrationTest, TestListenCanBeCalledMultipleTimes) {
740-
#if defined(FIREBASE_USE_STD_FUNCTION)
741724
class TestData {
742725
public:
743726
void SetDocumentSnapshot(const DocumentSnapshot& document_snapshot) {
@@ -781,7 +764,6 @@ TEST_F(FirestoreIntegrationTest, TestListenCanBeCalledMultipleTimes) {
781764

782765
EXPECT_THAT(test_data.WaitForDocumentSnapshot().GetData(),
783766
ContainerEq(MapFieldValue{{"foo", FieldValue::String("bar")}}));
784-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
785767
}
786768

787769
TEST_F(FirestoreIntegrationTest, TestDocumentSnapshotEventsNonExistent) {

‎firestore/integration_test_internal/src/includes_test.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ class IncludesTest : public testing::Test {
2020

2121
namespace {
2222

23-
struct TestListener : EventListener<int> {
24-
void OnEvent(const int&, Error, const std::string&) override {}
23+
struct TestListener {
24+
void OnEvent(const int&, Error, const std::string&) {}
2525
};
2626

2727
struct TestTransactionFunction : TransactionFunction {

‎firestore/integration_test_internal/src/integration_test.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -448,9 +448,6 @@ TEST_F(FirebaseFirestoreBasicTest, TestSetDelete) {
448448
}
449449

450450
TEST_F(FirebaseFirestoreBasicTest, TestDocumentListener) {
451-
SKIP_TEST_IF_USING_STLPORT; // STLPort uses EventListener<T> rather than
452-
// std::function.
453-
#if !defined(STLPORT)
454451
SignIn();
455452

456453
firebase::firestore::DocumentReference document = Doc();
@@ -487,7 +484,6 @@ TEST_F(FirebaseFirestoreBasicTest, TestDocumentListener) {
487484
{"val", firebase::firestore::FieldValue::String("start")}},
488485
firebase::firestore::MapFieldValue{
489486
{"val", firebase::firestore::FieldValue::String("update")}}));
490-
#endif // !defined(STLPORT)
491487
}
492488

493489
TEST_F(FirebaseFirestoreBasicTest, TestBatchWrite) {

‎firestore/integration_test_internal/src/server_timestamp_test.cc

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -248,31 +248,26 @@ TEST_F(ServerTimestampTest,
248248
}
249249

250250
TEST_F(ServerTimestampTest, TestServerTimestampsWorkViaTransactionSet) {
251-
#if defined(FIREBASE_USE_STD_FUNCTION)
252251
Await(TestFirestore()->RunTransaction(
253252
[this](Transaction& transaction, std::string&) -> Error {
254253
transaction.Set(doc_, set_data_);
255254
return Error::kErrorOk;
256255
}));
257256
VerifyTimestampsAreResolved(accumulator_.AwaitRemoteEvent());
258-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
259257
}
260258

261259
TEST_F(ServerTimestampTest, TestServerTimestampsWorkViaTransactionUpdate) {
262-
#if defined(FIREBASE_USE_STD_FUNCTION)
263260
WriteInitialData();
264261
Await(TestFirestore()->RunTransaction(
265262
[this](Transaction& transaction, std::string&) -> Error {
266263
transaction.Update(doc_, update_data_);
267264
return Error::kErrorOk;
268265
}));
269266
VerifyTimestampsAreResolved(accumulator_.AwaitRemoteEvent());
270-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
271267
}
272268

273269
TEST_F(ServerTimestampTest,
274270
TestServerTimestampsFailViaTransactionUpdateOnNonexistentDocument) {
275-
#if defined(FIREBASE_USE_STD_FUNCTION)
276271
Future<void> future = TestFirestore()->RunTransaction(
277272
[this](Transaction& transaction, std::string&) -> Error {
278273
transaction.Update(doc_, update_data_);
@@ -281,7 +276,6 @@ TEST_F(ServerTimestampTest,
281276
Await(future);
282277
EXPECT_EQ(FutureStatus::kFutureStatusComplete, future.status());
283278
EXPECT_EQ(Error::kErrorNotFound, future.error());
284-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
285279
}
286280

287281
TEST_F(ServerTimestampTest,

‎firestore/integration_test_internal/src/transaction_extra_test.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ namespace firestore {
2626

2727
using TransactionExtraTest = FirestoreIntegrationTest;
2828

29-
#if defined(FIREBASE_USE_STD_FUNCTION)
30-
3129
TEST_F(TransactionExtraTest,
3230
TestRetriesWhenDocumentThatWasReadWithoutBeingWrittenChanges) {
3331
DocumentReference doc1 = TestFirestore()->Collection("counter").Document();
@@ -105,7 +103,5 @@ TEST_F(TransactionExtraTest, TestReadingADocTwiceWithDifferentVersions) {
105103
DocumentSnapshot snapshot = ReadDocument(doc);
106104
}
107105

108-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
109-
110106
} // namespace firestore
111107
} // namespace firebase

‎firestore/integration_test_internal/src/transaction_test.cc

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ using ::testing::HasSubstr;
3939

4040
class TransactionTest : public FirestoreIntegrationTest {
4141
protected:
42-
#if defined(FIREBASE_USE_STD_FUNCTION)
4342
// We occasionally get transient error like "Could not reach Cloud Firestore
4443
// backend. Backend didn't respond within 10 seconds". Transaction requires
4544
// online and thus will not retry. So we do the retry in the testcase.
@@ -90,7 +89,6 @@ class TransactionTest : public FirestoreIntegrationTest {
9089
FAIL() << "Unexpected error code: " << error;
9190
}
9291
}
93-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
9492
};
9593

9694
class TestTransactionFunction : public TransactionFunction {
@@ -127,8 +125,6 @@ TEST_F(TransactionTest, TestGetNonexistentDocumentThenCreatePortableVersion) {
127125
snapshot.Get(transaction.key()));
128126
}
129127

130-
#if defined(FIREBASE_USE_STD_FUNCTION)
131-
132128
class TransactionStage {
133129
public:
134130
TransactionStage(
@@ -737,7 +733,5 @@ TEST_F(TransactionTest, TestCancellationOnError) {
737733
EXPECT_FALSE(snapshot.exists());
738734
}
739735

740-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
741-
742736
} // namespace firestore
743737
} // namespace firebase

‎firestore/integration_test_internal/src/validation_test.cc

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,6 @@ class ValidationTest : public FirestoreIntegrationTest {
9292
EXPECT_ERROR(TestFirestore()->batch().Update(document, data), reason);
9393
}
9494

95-
#if defined(FIREBASE_USE_STD_FUNCTION)
9695
Await(TestFirestore()->RunTransaction(
9796
[data, reason, include_sets, include_updates, document](
9897
Transaction& transaction, std::string& error_message) -> Error {
@@ -106,7 +105,6 @@ class ValidationTest : public FirestoreIntegrationTest {
106105
}
107106
return Error::kErrorOk;
108107
}));
109-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
110108
}
111109

112110
/**
@@ -339,7 +337,6 @@ TEST_F(ValidationTest, WritesMayContainIndirectlyNestedLists) {
339337
Await(document.Update(data));
340338
Await(TestFirestore()->batch().Update(document, data).Commit());
341339

342-
#if defined(FIREBASE_USE_STD_FUNCTION)
343340
Await(TestFirestore()->RunTransaction(
344341
[data, document, another_document](Transaction& transaction,
345342
std::string& error_message) -> Error {
@@ -349,7 +346,6 @@ TEST_F(ValidationTest, WritesMayContainIndirectlyNestedLists) {
349346
transaction.Set(another_document, data);
350347
return Error::kErrorOk;
351348
}));
352-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
353349
}
354350

355351
// TODO(zxu): There is no way to create Firestore with different project id yet.

‎firestore/src/android/document_reference_android.cc

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,6 @@ Future<void> DocumentReferenceInternal::Delete() {
163163
return promises_.NewFuture<void>(env, AsyncFn::kDelete, task);
164164
}
165165

166-
#if defined(FIREBASE_USE_STD_FUNCTION)
167-
168166
ListenerRegistration DocumentReferenceInternal::AddSnapshotListener(
169167
MetadataChanges metadata_changes,
170168
std::function<void(const DocumentSnapshot&, Error, const std::string&)>
@@ -175,8 +173,7 @@ ListenerRegistration DocumentReferenceInternal::AddSnapshotListener(
175173
/*passing_listener_ownership=*/true);
176174
}
177175

178-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
179-
176+
// TODO(b/191969080): Remove the passing_listener_ownership if possible.
180177
ListenerRegistration DocumentReferenceInternal::AddSnapshotListener(
181178
MetadataChanges metadata_changes,
182179
EventListener<DocumentSnapshot>* listener,

‎firestore/src/android/document_reference_android.h

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,6 @@ class DocumentReferenceInternal : public Wrapper {
132132
*/
133133
Future<void> Delete();
134134

135-
#if defined(FIREBASE_USE_STD_FUNCTION)
136135
/**
137136
* @brief Starts listening to the document referenced by this
138137
* DocumentReference.
@@ -144,15 +143,11 @@ class DocumentReferenceInternal : public Wrapper {
144143
* called, snapshot value is valid if and only if error is Error::kErrorOk.
145144
*
146145
* @return A registration object that can be used to remove the listener.
147-
*
148-
* @note This method is not available when using the STLPort C++ runtime
149-
* library.
150146
*/
151147
ListenerRegistration AddSnapshotListener(
152148
MetadataChanges metadata_changes,
153149
std::function<void(const DocumentSnapshot&, Error, const std::string&)>
154150
callback);
155-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
156151

157152
/**
158153
* Starts listening to the document referenced by this DocumentReference.

‎firestore/src/android/event_listener_android.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
#define FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_EVENT_LISTENER_ANDROID_H_
55

66
#include "firestore/src/android/firestore_android.h"
7+
#include "firestore/src/common/event_listener.h"
78
#include "firestore/src/include/firebase/firestore/document_snapshot.h"
8-
#include "firestore/src/include/firebase/firestore/event_listener.h"
99
#include "firestore/src/jni/jni_fwd.h"
1010

1111
namespace firebase {

‎firestore/src/android/firestore_android.cc

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -448,23 +448,17 @@ Future<void> FirestoreInternal::RunTransaction(TransactionFunction* update,
448448

449449
if (!env.ok()) return {};
450450

451-
#if defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
452451
auto* completion =
453452
static_cast<LambdaTransactionFunction*>(is_lambda ? update : nullptr);
454453
return promises_->NewFuture<void>(env, AsyncFn::kRunTransaction, task,
455454
completion);
456-
#else // defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
457-
return promises_->NewFuture<void>(env, AsyncFn::kRunTransaction, task);
458-
#endif // defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
459455
}
460456

461-
#if defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
462457
Future<void> FirestoreInternal::RunTransaction(
463458
std::function<Error(Transaction&, std::string&)> update) {
464459
auto* lambda_update = new LambdaTransactionFunction(Move(update));
465460
return RunTransaction(lambda_update, /*is_lambda=*/true);
466461
}
467-
#endif // defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
468462

469463
Future<void> FirestoreInternal::DisableNetwork() {
470464
Env env = GetEnv();
@@ -510,17 +504,13 @@ ListenerRegistration FirestoreInternal::AddSnapshotsInSyncListener(
510504
this, listener, passing_listener_ownership, java_registration));
511505
}
512506

513-
#if defined(FIREBASE_USE_STD_FUNCTION)
514-
515507
ListenerRegistration FirestoreInternal::AddSnapshotsInSyncListener(
516508
std::function<void()> callback) {
517509
auto* listener = new LambdaEventListener<void>(firebase::Move(callback));
518510
return AddSnapshotsInSyncListener(listener,
519511
/*passing_listener_ownership=*/true);
520512
}
521513

522-
#endif // defined(FIREBASE_USE_STD_FUNCTION)
523-
524514
void FirestoreInternal::RegisterListenerRegistration(
525515
ListenerRegistrationInternal* registration) {
526516
MutexLock lock(listener_registration_mutex_);

0 commit comments

Comments
 (0)