Skip to content

Commit 61194be

Browse files
wilhuffCorrob
authored andcommitted
Port FSTListenerRegistration to C++ (#2619)
* Replace FSTQueryListener with C++ QueryListener * objc::unordered_map * Add a lint check for Objective-C banned words
1 parent bfd5509 commit 61194be

8 files changed

+168
-48
lines changed

Firestore/Source/API/FIRDocumentReference.mm

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -212,8 +212,9 @@ - (void)getDocumentWithSource:(FIRFirestoreSource)source
212212
- (id<FIRListenerRegistration>)addSnapshotListenerInternalWithOptions:(ListenOptions)internalOptions
213213
listener:(FIRDocumentSnapshotBlock)
214214
listener {
215-
return _documentReference.AddSnapshotListener([self wrapDocumentSnapshotBlock:listener],
216-
std::move(internalOptions));
215+
ListenerRegistration result = _documentReference.AddSnapshotListener(
216+
[self wrapDocumentSnapshotBlock:listener], std::move(internalOptions));
217+
return [[FSTListenerRegistration alloc] initWithRegistration:std::move(result)];
217218
}
218219

219220
- (StatusOrCallback<DocumentSnapshot>)wrapDocumentSnapshotBlock:(FIRDocumentSnapshotBlock)block {

Firestore/Source/API/FIRListenerRegistration+Internal.h

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -16,23 +16,16 @@
1616

1717
#import "FIRListenerRegistration.h"
1818

19-
#include <memory>
20-
21-
#include "Firestore/core/src/firebase/firestore/core/query_listener.h"
22-
23-
@class FSTAsyncQueryListener;
24-
@class FSTFirestoreClient;
19+
#include "Firestore/core/src/firebase/firestore/api/listener_registration.h"
2520

2621
NS_ASSUME_NONNULL_BEGIN
2722

28-
using firebase::firestore::core::QueryListener;
23+
using firebase::firestore::api::ListenerRegistration;
2924

3025
/** Private implementation of the FIRListenerRegistration protocol. */
3126
@interface FSTListenerRegistration : NSObject <FIRListenerRegistration>
3227

33-
- (instancetype)initWithClient:(FSTFirestoreClient *)client
34-
asyncListener:(FSTAsyncQueryListener *)asyncListener
35-
internalListener:(std::shared_ptr<QueryListener>)internalListener;
28+
- (instancetype)initWithRegistration:(ListenerRegistration &&)registration;
3629

3730
@end
3831

Firestore/Source/API/FIRListenerRegistration.mm

Lines changed: 7 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,43 +18,25 @@
1818

1919
#import "Firestore/Source/API/FIRListenerRegistration+Internal.h"
2020

21-
#import "Firestore/Source/Core/FSTFirestoreClient.h"
22-
#import "Firestore/Source/Util/FSTAsyncQueryListener.h"
21+
#include "Firestore/core/src/firebase/firestore/util/delayed_constructor.h"
2322

2423
NS_ASSUME_NONNULL_BEGIN
2524

26-
@implementation FSTListenerRegistration {
27-
/** The client that was used to register this listen. */
28-
FSTFirestoreClient *_client;
29-
30-
/** The async listener that is used to mute events synchronously. */
31-
FSTAsyncQueryListener *_asyncListener;
25+
using firebase::firestore::util::DelayedConstructor;
3226

33-
/** The internal QueryListener that can be used to unlisten the query. */
34-
std::weak_ptr<QueryListener> _internalListener;
27+
@implementation FSTListenerRegistration {
28+
DelayedConstructor<ListenerRegistration> _registration;
3529
}
3630

37-
- (instancetype)initWithClient:(FSTFirestoreClient *)client
38-
asyncListener:(FSTAsyncQueryListener *)asyncListener
39-
internalListener:(std::shared_ptr<QueryListener>)internalListener {
31+
- (instancetype)initWithRegistration:(ListenerRegistration &&)registration {
4032
if (self = [super init]) {
41-
_client = client;
42-
_asyncListener = asyncListener;
43-
_internalListener = internalListener;
33+
_registration.Init(std::move(registration));
4434
}
4535
return self;
4636
}
4737

4838
- (void)remove {
49-
[_asyncListener mute];
50-
51-
std::shared_ptr<QueryListener> listener = _internalListener.lock();
52-
if (listener) {
53-
[_client removeListener:listener];
54-
listener.reset();
55-
_internalListener.reset();
56-
}
57-
_asyncListener = nil;
39+
_registration->Remove();
5840
}
5941

6042
@end

Firestore/Source/API/FIRQuery.mm

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -199,9 +199,10 @@ ListenOptions listenOptions(
199199
[firestore->client() listenToQuery:query
200200
options:internalOptions
201201
viewSnapshotHandler:[asyncListener asyncSnapshotHandler]];
202-
return [[FSTListenerRegistration alloc] initWithClient:self.firestore.client
203-
asyncListener:asyncListener
204-
internalListener:internalListener];
202+
203+
return [[FSTListenerRegistration alloc]
204+
initWithRegistration:ListenerRegistration(firestore->client(), asyncListener,
205+
std::move(internalListener))];
205206
}
206207

207208
- (FIRQuery *)queryWhereField:(NSString *)field isEqualTo:(id)value {

Firestore/core/src/firebase/firestore/api/document_reference.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,9 @@
2929

3030
#import "FIRDocumentReference.h"
3131
#import "FIRFirestoreSource.h"
32-
#import "FIRListenerRegistration.h"
3332

3433
#include "Firestore/core/src/firebase/firestore/api/document_snapshot.h"
34+
#include "Firestore/core/src/firebase/firestore/api/listener_registration.h"
3535
#include "Firestore/core/src/firebase/firestore/core/listen_options.h"
3636
#include "Firestore/core/src/firebase/firestore/model/document_key.h"
3737
#include "Firestore/core/src/firebase/firestore/model/resource_path.h"
@@ -89,7 +89,7 @@ class DocumentReference {
8989
void GetDocument(FIRFirestoreSource source,
9090
util::StatusOrCallback<DocumentSnapshot>&& completion);
9191

92-
id<FIRListenerRegistration> AddSnapshotListener(
92+
ListenerRegistration AddSnapshotListener(
9393
util::StatusOrCallback<DocumentSnapshot>&& listener,
9494
core::ListenOptions options);
9595

Firestore/core/src/firebase/firestore/api/document_reference.mm

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -127,12 +127,16 @@ ListenOptions options(
127127
/*include_document_metadata_changes=*/true,
128128
/*wait_for_sync_when_online=*/true);
129129

130+
// Create an empty ListenerRegistration captured in the closure below and
131+
// then, once the listen has started, assign to it and signal the listener
132+
// that we succeeded.
133+
//
130134
// TODO(varconst): replace with a synchronization primitive that doesn't
131135
// require libdispatch. See
132136
// https://github.com/firebase/firebase-ios-sdk/blob/3ccbdcdc65c93c4621c045c3c6d15de9dcefa23f/Firestore/Source/Core/FSTFirestoreClient.mm#L161
133137
// for an example.
134138
dispatch_semaphore_t registered = dispatch_semaphore_create(0);
135-
auto listener_registration = std::make_shared<id<FIRListenerRegistration>>();
139+
auto listener_registration = std::make_shared<ListenerRegistration>();
136140
StatusOrCallback<DocumentSnapshot> listener =
137141
[listener_registration, registered, completion,
138142
source](StatusOr<DocumentSnapshot> maybe_snapshot) {
@@ -146,7 +150,7 @@ ListenOptions options(
146150
// Remove query first before passing event to user to avoid user actions
147151
// affecting the now stale query.
148152
dispatch_semaphore_wait(registered, DISPATCH_TIME_FOREVER);
149-
[*listener_registration remove];
153+
listener_registration->Remove();
150154

151155
if (!snapshot.exists() && snapshot.metadata().from_cache()) {
152156
// TODO(dimond): Reconsider how to raise missing documents when
@@ -178,7 +182,7 @@ ListenOptions options(
178182
dispatch_semaphore_signal(registered);
179183
}
180184

181-
id<FIRListenerRegistration> DocumentReference::AddSnapshotListener(
185+
ListenerRegistration DocumentReference::AddSnapshotListener(
182186
StatusOrCallback<DocumentSnapshot>&& listener, ListenOptions options) {
183187
Firestore* firestore = firestore_;
184188
FSTQuery* query = [FSTQuery queryWithPath:key_.path()];
@@ -215,9 +219,8 @@ ListenOptions options(
215219
listenToQuery:query
216220
options:options
217221
viewSnapshotHandler:[async_listener asyncSnapshotHandler]];
218-
return [[FSTListenerRegistration alloc] initWithClient:firestore_->client()
219-
asyncListener:async_listener
220-
internalListener:internal_listener];
222+
return ListenerRegistration(firestore_->client(), async_listener,
223+
internal_listener);
221224
}
222225

223226
bool operator==(const DocumentReference& lhs, const DocumentReference& rhs) {
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
/*
2+
* Copyright 2019 Google
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_API_LISTENER_REGISTRATION_H_
18+
#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_API_LISTENER_REGISTRATION_H_
19+
20+
#if !defined(__OBJC__)
21+
#error "This header only supports Objective-C++"
22+
#endif // !defined(__OBJC__)
23+
24+
#import <Foundation/Foundation.h>
25+
26+
#include <memory>
27+
#include <utility>
28+
29+
#include "Firestore/core/src/firebase/firestore/core/query_listener.h"
30+
31+
@class FSTAsyncQueryListener;
32+
@class FSTFirestoreClient;
33+
34+
NS_ASSUME_NONNULL_BEGIN
35+
36+
namespace firebase {
37+
namespace firestore {
38+
namespace api {
39+
40+
/**
41+
* An internal handle that encapsulates a user's ability to request that we
42+
* stop listening to a query. When a user calls Remove(), ListenerRegistration
43+
* will synchronously mute the listener and then send a request to the
44+
* FirestoreClient to actually unlisten.
45+
*
46+
* ListenerRegistration will not automaticlaly stop listening if it is
47+
* destroyed. We allow users to fire and forget listens if they never want to
48+
* stop them.
49+
*
50+
* Getting shutdown code right is tricky so ListenerRegistration is very
51+
* forgiving. It will tolerate:
52+
*
53+
* * Multiple calls to Remove(),
54+
* * calls to Remove() after we send an error,
55+
* * calls to Remove() even after deleting the App in which the listener was
56+
* started.
57+
*
58+
* ListenerRegistration is default constructible to facilitate the pattern in
59+
* DocumentReference::GetDocument, where the closure that implements a listener
60+
* needs to be able to use the ListenerRegistration thats returned from
61+
* starting the listener. The default ListenerRegistration acts as a shared
62+
* placeholder that's filled in later once the listener is started.
63+
*/
64+
class ListenerRegistration {
65+
public:
66+
ListenerRegistration() = default;
67+
68+
ListenerRegistration(FSTFirestoreClient* client,
69+
FSTAsyncQueryListener* async_listener,
70+
std::shared_ptr<core::QueryListener> internal_listener)
71+
: client_(client),
72+
async_listener_(async_listener),
73+
internal_listener_(std::move(internal_listener)) {
74+
}
75+
76+
/**
77+
* Removes the listener being tracked by this FIRListenerRegistration. After
78+
* the initial call, subsequent calls have no effect.
79+
*/
80+
void Remove();
81+
82+
private:
83+
/** The client that was used to register this listen. */
84+
FSTFirestoreClient* client_ = nil;
85+
86+
/** The async listener that is used to mute events synchronously. */
87+
FSTAsyncQueryListener* async_listener_ = nil;
88+
89+
/** The internal QueryListener that can be used to unlisten the query. */
90+
std::weak_ptr<core::QueryListener> internal_listener_;
91+
};
92+
93+
} // namespace api
94+
} // namespace firestore
95+
} // namespace firebase
96+
97+
NS_ASSUME_NONNULL_END
98+
99+
#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_API_LISTENER_REGISTRATION_H_
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright 2019 Google
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#include "Firestore/core/src/firebase/firestore/api/listener_registration.h"
18+
19+
#import "Firestore/Source/Core/FSTFirestoreClient.h"
20+
#import "Firestore/Source/Util/FSTAsyncQueryListener.h"
21+
22+
namespace firebase {
23+
namespace firestore {
24+
namespace api {
25+
26+
void ListenerRegistration::Remove() {
27+
[async_listener_ mute];
28+
async_listener_ = nil;
29+
30+
std::shared_ptr<QueryListener> listener = internal_listener_.lock();
31+
if (listener) {
32+
[client_ removeListener:listener];
33+
listener.reset();
34+
internal_listener_.reset();
35+
client_ = nil;
36+
}
37+
}
38+
39+
} // namespace api
40+
} // namespace firestore
41+
} // namespace firebase

0 commit comments

Comments
 (0)