Skip to content

Commit b1c3695

Browse files
wilhuffa-maurice
wilhuff
authored and
a-maurice
committed
Split PromiseFactory out of WrapperFuture
This makes promise creation more closely match iOS and paves the way for removing WrapperFuture altogether. Use PromiseFactory in all internal type implementations instead of extending WrapperFuture and then dDelete WrapperFuture. PiperOrigin-RevId: 319025783
1 parent fef5d47 commit b1c3695

13 files changed

+126
-113
lines changed

‎firestore/CMakeLists.txt‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ set(android_SRCS
113113
src/android/util_android.h
114114
src/android/wrapper.cc
115115
src/android/wrapper.h
116-
src/android/wrapper_future.h
117116
src/android/write_batch_android.cc
118117
src/android/write_batch_android.h)
119118

‎firestore/src/android/collection_reference_android.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,15 +114,15 @@ Future<DocumentReference> CollectionReferenceInternal::Add(
114114
map_value.java_object());
115115
CheckAndClearJniExceptions(env);
116116

117-
auto promise = MakePromise<DocumentReference>();
117+
auto promise = promises_.MakePromise<DocumentReference>();
118118
promise.RegisterForTask(CollectionReferenceFn::kAdd, task);
119119
env->DeleteLocalRef(task);
120120
CheckAndClearJniExceptions(env);
121121
return promise.GetFuture();
122122
}
123123

124124
Future<DocumentReference> CollectionReferenceInternal::AddLastResult() {
125-
return LastResult<DocumentReference>(CollectionReferenceFn::kAdd);
125+
return promises_.LastResult<DocumentReference>(CollectionReferenceFn::kAdd);
126126
}
127127

128128
/* static */

‎firestore/src/android/collection_reference_android.h‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55

66
#include "firestore/src/android/firestore_android.h"
77
#include "firestore/src/android/query_android.h"
8-
#include "firestore/src/android/wrapper_future.h"
98

109
namespace firebase {
1110
namespace firestore {

‎firestore/src/android/document_reference_android.cc‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,15 +119,15 @@ Future<DocumentSnapshot> DocumentReferenceInternal::Get(Source source) {
119119
SourceInternal::ToJavaObject(env, source));
120120
CheckAndClearJniExceptions(env);
121121

122-
auto promise = MakePromise<DocumentSnapshot>();
122+
auto promise = promises_.MakePromise<DocumentSnapshot>();
123123
promise.RegisterForTask(DocumentReferenceFn::kGet, task);
124124
env->DeleteLocalRef(task);
125125
CheckAndClearJniExceptions(env);
126126
return promise.GetFuture();
127127
}
128128

129129
Future<DocumentSnapshot> DocumentReferenceInternal::GetLastResult() {
130-
return LastResult<DocumentSnapshot>(DocumentReferenceFn::kGet);
130+
return promises_.LastResult<DocumentSnapshot>(DocumentReferenceFn::kGet);
131131
}
132132

133133
Future<void> DocumentReferenceInternal::Set(const MapFieldValue& data,
@@ -142,15 +142,15 @@ Future<void> DocumentReferenceInternal::Set(const MapFieldValue& data,
142142
env->DeleteLocalRef(java_options);
143143
CheckAndClearJniExceptions(env);
144144

145-
auto promise = MakePromise<void>();
145+
auto promise = promises_.MakePromise<void>();
146146
promise.RegisterForTask(DocumentReferenceFn::kSet, task);
147147
env->DeleteLocalRef(task);
148148
CheckAndClearJniExceptions(env);
149149
return promise.GetFuture();
150150
}
151151

152152
Future<void> DocumentReferenceInternal::SetLastResult() {
153-
return LastResult<void>(DocumentReferenceFn::kSet);
153+
return promises_.LastResult<void>(DocumentReferenceFn::kSet);
154154
}
155155

156156
Future<void> DocumentReferenceInternal::Update(const MapFieldValue& data) {
@@ -161,7 +161,7 @@ Future<void> DocumentReferenceInternal::Update(const MapFieldValue& data) {
161161
map_value.java_object());
162162
CheckAndClearJniExceptions(env);
163163

164-
auto promise = MakePromise<void>();
164+
auto promise = promises_.MakePromise<void>();
165165
promise.RegisterForTask(DocumentReferenceFn::kUpdate, task);
166166
env->DeleteLocalRef(task);
167167
CheckAndClearJniExceptions(env);
@@ -190,15 +190,15 @@ Future<void> DocumentReferenceInternal::Update(const MapFieldPathValue& data) {
190190
env->DeleteLocalRef(more_fields_and_values);
191191
CheckAndClearJniExceptions(env);
192192

193-
auto promise = MakePromise<void>();
193+
auto promise = promises_.MakePromise<void>();
194194
promise.RegisterForTask(DocumentReferenceFn::kUpdate, task);
195195
env->DeleteLocalRef(task);
196196
CheckAndClearJniExceptions(env);
197197
return promise.GetFuture();
198198
}
199199

200200
Future<void> DocumentReferenceInternal::UpdateLastResult() {
201-
return LastResult<void>(DocumentReferenceFn::kUpdate);
201+
return promises_.LastResult<void>(DocumentReferenceFn::kUpdate);
202202
}
203203

204204
Future<void> DocumentReferenceInternal::Delete() {
@@ -207,15 +207,15 @@ Future<void> DocumentReferenceInternal::Delete() {
207207
obj_, document_reference::GetMethodId(document_reference::kDelete));
208208
CheckAndClearJniExceptions(env);
209209

210-
auto promise = MakePromise<void>();
210+
auto promise = promises_.MakePromise<void>();
211211
promise.RegisterForTask(DocumentReferenceFn::kDelete, task);
212212
env->DeleteLocalRef(task);
213213
CheckAndClearJniExceptions(env);
214214
return promise.GetFuture();
215215
}
216216

217217
Future<void> DocumentReferenceInternal::DeleteLastResult() {
218-
return LastResult<void>(DocumentReferenceFn::kDelete);
218+
return promises_.LastResult<void>(DocumentReferenceFn::kDelete);
219219
}
220220

221221
#if defined(FIREBASE_USE_STD_FUNCTION)

‎firestore/src/android/document_reference_android.h‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77

88
#include "app/src/reference_counted_future_impl.h"
99
#include "firestore/src/android/firestore_android.h"
10-
#include "firestore/src/android/wrapper_future.h"
10+
#include "firestore/src/android/promise_factory_android.h"
11+
#include "firestore/src/android/wrapper.h"
1112
#include "firestore/src/include/firebase/firestore/collection_reference.h"
1213

1314
namespace firebase {
@@ -16,9 +17,9 @@ namespace firestore {
1617
class Firestore;
1718

1819
// Each API of DocumentReference that returns a Future needs to define an enum
19-
// value here. For example, Foo() and FooLastResult() implementation relies on
20-
// the enum value kFoo. The enum values are used to identify and manage Future
21-
// in the Firestore Future manager.
20+
// value here. For example, a Future-returning method Foo() relies on the enum
21+
// value kFoo. The enum values are used to identify and manage Future in the
22+
// Firestore Future manager.
2223
enum class DocumentReferenceFn {
2324
kGet = 0,
2425
kSet,
@@ -28,11 +29,12 @@ enum class DocumentReferenceFn {
2829
};
2930

3031
// This is the Android implementation of DocumentReference.
31-
class DocumentReferenceInternal
32-
: public WrapperFuture<DocumentReferenceFn, DocumentReferenceFn::kCount> {
32+
class DocumentReferenceInternal : public Wrapper {
3333
public:
3434
using ApiType = DocumentReference;
35-
using WrapperFuture::WrapperFuture;
35+
36+
DocumentReferenceInternal(FirestoreInternal* firestore, jobject object)
37+
: Wrapper(firestore, object), promises_(firestore) {}
3638

3739
/** Gets the Firestore instance associated with this document reference. */
3840
Firestore* firestore();
@@ -209,6 +211,8 @@ class DocumentReferenceInternal
209211
static bool Initialize(App* app);
210212
static void Terminate(App* app);
211213

214+
PromiseFactory<DocumentReferenceFn> promises_;
215+
212216
// Below are cached call results.
213217
mutable std::string cached_id_;
214218
mutable std::string cached_path_;

‎firestore/src/android/promise_android.h‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,13 @@ namespace firebase {
1414
namespace firestore {
1515

1616
// This class simplifies the implementation of Future APIs for Android wrappers.
17-
// PublicType is the public type, say Foo, and InternalType is FooInternal,
18-
// which is required to be a subclass of WrapperFuture. FnEnumType is an enum
19-
// class that defines a set of APIs returning a Future. For example, to
20-
// implement Future<DocumentReference> CollectionReferenceInternal::Add(),
17+
// PublicType is the public type, say Foo, and InternalType is FooInternal.
18+
// FnEnumType is an enum class that defines a set of APIs returning a Future.
19+
//
20+
// For example, to implement:
21+
//
22+
// Future<DocumentReference> CollectionReferenceInternal::Add()
23+
//
2124
// PublicType is DocumentReference, InternalType is DocumentReferenceInternal,
2225
// and FnEnumType is CollectionReferenceFn.
2326
template <typename PublicType, typename InternalType, typename FnEnumType>
@@ -30,7 +33,7 @@ class Promise {
3033
template <typename PublicT>
3134
class Completion {
3235
public:
33-
virtual ~Completion() {}
36+
virtual ~Completion() = default;
3437
virtual void CompleteWith(Error error_code, const char* error_message,
3538
PublicT* result) = 0;
3639
};
@@ -65,7 +68,7 @@ class Promise {
6568
FirestoreInternal* firestore, Completion<PublicT>* completion)
6669
: impl_{impl}, firestore_{firestore}, completion_(completion) {}
6770

68-
virtual ~CompleterBase() {}
71+
virtual ~CompleterBase() = default;
6972

7073
FirestoreInternal* firestore() { return firestore_; }
7174

@@ -97,7 +100,6 @@ class Promise {
97100
error_code = Error::kErrorCancelled;
98101
break;
99102
default:
100-
error_code = Error::kErrorUnknown;
101103
FIREBASE_ASSERT_MESSAGE(false, "unknown FutureResult %d",
102104
result_code);
103105
break;

‎firestore/src/android/promise_factory_android.h‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
#ifndef FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_PROMISE_FACTORY_ANDROID_H_
2+
#define FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_PROMISE_FACTORY_ANDROID_H_
3+
4+
#include "firestore/src/android/promise_android.h"
5+
#include "firestore/src/common/type_mapping.h"
6+
7+
namespace firebase {
8+
namespace firestore {
9+
10+
// Wraps a `FutureManager` and allows creating `Promise`s. `EnumT` must be an
11+
// enumeration that lists the async API methods each of which must be backed by
12+
// a future; it is expected to contain a member called `kCount` that stands for
13+
// the total number of the async APIs.
14+
template <typename EnumT>
15+
class PromiseFactory {
16+
public:
17+
explicit PromiseFactory(FirestoreInternal* firestore)
18+
: firestore_(firestore) {
19+
firestore_->future_manager().AllocFutureApi(this, ApiCount());
20+
}
21+
22+
PromiseFactory(const PromiseFactory& rhs) : firestore_(rhs.firestore_) {
23+
firestore_->future_manager().AllocFutureApi(this, ApiCount());
24+
}
25+
26+
PromiseFactory(PromiseFactory&& rhs) : firestore_(rhs.firestore_) {
27+
firestore_->future_manager().MoveFutureApi(&rhs, this);
28+
}
29+
30+
~PromiseFactory() { firestore_->future_manager().ReleaseFutureApi(this); }
31+
32+
PromiseFactory& operator=(const PromiseFactory&) = delete;
33+
PromiseFactory& operator=(PromiseFactory&&) = delete;
34+
35+
// Creates a Promise representing the completion of an underlying Java Task.
36+
// This can be used to implement APIs that return Futures of some public type.
37+
// Use MakePromise<void, void>() to create a Future<void>.
38+
template <typename PublicT, typename InternalT = InternalType<PublicT>>
39+
Promise<PublicT, InternalT, EnumT> MakePromise() {
40+
return Promise<PublicT, InternalT, EnumT>{future_api(), firestore_};
41+
}
42+
43+
// A helper that generalizes the logic for FooLastResult() of each Foo()
44+
// defined.
45+
template <typename ResultType>
46+
Future<ResultType> LastResult(EnumT index) {
47+
const auto& result = future_api()->LastResult(static_cast<int>(index));
48+
return static_cast<const Future<ResultType>&>(result);
49+
}
50+
51+
private:
52+
// Gets the reference-counted Future implementation of this instance, which
53+
// can be used to create a Future.
54+
ReferenceCountedFutureImpl* future_api() {
55+
return firestore_->future_manager().GetFutureApi(this);
56+
}
57+
58+
constexpr int ApiCount() const { return static_cast<int>(EnumT::kCount); }
59+
60+
FirestoreInternal* firestore_ = nullptr;
61+
};
62+
63+
} // namespace firestore
64+
} // namespace firebase
65+
66+
#endif // FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_PROMISE_FACTORY_ANDROID_H_

‎firestore/src/android/query_android.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -79,15 +79,15 @@ Future<QuerySnapshot> QueryInternal::Get(Source source) {
7979
SourceInternal::ToJavaObject(env, source));
8080
CheckAndClearJniExceptions(env);
8181

82-
auto promise = MakePromise<QuerySnapshot>();
82+
auto promise = promises_.MakePromise<QuerySnapshot>();
8383
promise.RegisterForTask(QueryFn::kGet, task);
8484
env->DeleteLocalRef(task);
8585
CheckAndClearJniExceptions(env);
8686
return promise.GetFuture();
8787
}
8888

8989
Future<QuerySnapshot> QueryInternal::GetLastResult() {
90-
return LastResult<QuerySnapshot>(QueryFn::kGet);
90+
return promises_.LastResult<QuerySnapshot>(QueryFn::kGet);
9191
}
9292

9393
/* static */

‎firestore/src/android/query_android.h‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,13 @@
22
#define FIREBASE_FIRESTORE_CLIENT_CPP_SRC_ANDROID_QUERY_ANDROID_H_
33

44
#include <jni.h>
5+
56
#include <cstdint>
67

78
#include "app/src/reference_counted_future_impl.h"
89
#include "app/src/util_android.h"
9-
#include "firestore/src/android/wrapper_future.h"
10+
#include "firestore/src/android/promise_factory_android.h"
11+
#include "firestore/src/android/wrapper.h"
1012
#include "firestore/src/include/firebase/firestore/field_path.h"
1113
#include "firestore/src/include/firebase/firestore/query.h"
1214

@@ -16,9 +18,9 @@ namespace firestore {
1618
class Firestore;
1719

1820
// Each API of Query that returns a Future needs to define an enum value here.
19-
// For example, Foo() and FooLastResult() implementation relies on the enum
20-
// value kFoo. The enum values are used to identify and manage Future in the
21-
// Firestore Future manager.
21+
// For example, a Future-returning method Foo() relies on the enum value kFoo.
22+
// The enum values are used to identify and manage Future in the Firestore
23+
// Future manager.
2224
enum class QueryFn {
2325
// Enum values for the baseclass Query.
2426
kGet = 0,
@@ -95,10 +97,12 @@ enum class QueryFn {
9597

9698
METHOD_LOOKUP_DECLARATION(query, QUERY_METHODS)
9799

98-
class QueryInternal : public WrapperFuture<QueryFn, QueryFn::kCount> {
100+
class QueryInternal : public Wrapper {
99101
public:
100102
using ApiType = Query;
101-
using WrapperFuture::WrapperFuture;
103+
104+
QueryInternal(FirestoreInternal* firestore, jobject object)
105+
: Wrapper(firestore, object), promises_(firestore) {}
102106

103107
/** Gets the Firestore instance associated with this query. */
104108
Firestore* firestore();
@@ -433,6 +437,9 @@ class QueryInternal : public WrapperFuture<QueryFn, QueryFn::kCount> {
433437
MetadataChanges metadata_changes, EventListener<QuerySnapshot>* listener,
434438
bool passing_listener_ownership = false);
435439

440+
protected:
441+
PromiseFactory<QueryFn> promises_;
442+
436443
private:
437444
friend class FirestoreInternal;
438445

‎firestore/src/android/transaction_android.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
#include "app/memory/shared_ptr.h"
77
#include "app/meta/move.h"
88
#include "app/src/embedded_file.h"
9-
#include "firestore/src/android/wrapper_future.h"
9+
#include "firestore/src/android/wrapper.h"
1010
#include "firestore/src/include/firebase/firestore/document_reference.h"
1111
#include "firestore/src/include/firebase/firestore/map_field_value.h"
1212
#include "firestore/src/include/firebase/firestore/transaction.h"

0 commit comments

Comments
 (0)