Skip to content

Commit 148035f

Browse files
authored
Remove accessors for prioritizer internals (#5352)
* Remove accessors for prioritizer internals This effectively makes it impossible to access the event sets in an unsynchronized way. Fixes #5312 * Fix typoo
1 parent c57ae0f commit 148035f

File tree

4 files changed

+58
-31
lines changed

4 files changed

+58
-31
lines changed

GoogleDataTransportCCTSupport/GDTCCTLibrary/GDTCCTPrioritizer.m

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,20 @@
4040
return archivePath;
4141
}
4242

43+
/** This class extension is for declaring private properties. */
44+
@interface GDTCCTPrioritizer ()
45+
46+
/** All CCT events that have been processed by this prioritizer. */
47+
@property(nonatomic) NSMutableSet<GDTCOREvent *> *CCTEvents;
48+
49+
/** All FLL events that have been processed by this prioritizer. */
50+
@property(nonatomic) NSMutableSet<GDTCOREvent *> *FLLEvents;
51+
52+
/** All CSH events that have been processed by this prioritizer. */
53+
@property(nonatomic) NSMutableSet<GDTCOREvent *> *CSHEvents;
54+
55+
@end
56+
4357
@implementation GDTCCTPrioritizer
4458

4559
+ (void)load {
@@ -73,6 +87,29 @@ - (instancetype)init {
7387
return self;
7488
}
7589

90+
- (nullable NSSet *)eventsForTarget:(GDTCORTarget)target {
91+
__block NSSet *events;
92+
dispatch_sync(_queue, ^{
93+
switch (target) {
94+
case kGDTCORTargetCCT:
95+
events = [self->_CCTEvents copy];
96+
break;
97+
98+
case kGDTCORTargetFLL:
99+
events = [self->_FLLEvents copy];
100+
break;
101+
102+
case kGDTCORTargetCSH:
103+
events = [self->_CSHEvents copy];
104+
break;
105+
106+
default:
107+
break;
108+
}
109+
});
110+
return events;
111+
}
112+
76113
#pragma mark - GDTCORPrioritizer Protocol
77114

78115
- (void)prioritizeEvent:(GDTCOREvent *)event {

GoogleDataTransportCCTSupport/GDTCCTLibrary/GDTCCTUploader.m

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -276,13 +276,10 @@ - (void)uploadPackage:(GDTCORUploadPackage *)package {
276276

277277
- (BOOL)readyToUploadTarget:(GDTCORTarget)target conditions:(GDTCORUploadConditions)conditions {
278278
__block BOOL result = NO;
279+
NSSet *CSHEvents = [[GDTCCTPrioritizer sharedInstance] eventsForTarget:kGDTCORTargetCSH];
279280
dispatch_sync(_uploaderQueue, ^{
280281
if (target == kGDTCORTargetCSH) {
281-
if ([GDTCCTPrioritizer sharedInstance].CSHEvents.count > 0) {
282-
result = YES;
283-
} else {
284-
result = NO;
285-
}
282+
result = CSHEvents.count > 0;
286283
return;
287284
}
288285

GoogleDataTransportCCTSupport/GDTCCTLibrary/Private/GDTCCTPrioritizer.h

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,6 @@ NS_ASSUME_NONNULL_BEGIN
2929
/** The queue on which this prioritizer operates. */
3030
@property(nonatomic) dispatch_queue_t queue;
3131

32-
/** All CCT events that have been processed by this prioritizer. */
33-
@property(nonatomic) NSMutableSet<GDTCOREvent *> *CCTEvents;
34-
35-
/** All FLL events that have been processed by this prioritizer. */
36-
@property(nonatomic) NSMutableSet<GDTCOREvent *> *FLLEvents;
37-
38-
/** All CSH events that have been processed by this prioritizer. */
39-
@property(nonatomic) NSMutableSet<GDTCOREvent *> *CSHEvents;
40-
4132
/** The most recent attempted upload of CCT daily uploaded logs. */
4233
@property(nonatomic) GDTCORClock *CCTTimeOfLastDailyUpload;
4334

@@ -50,6 +41,13 @@ NS_ASSUME_NONNULL_BEGIN
5041
*/
5142
+ (instancetype)sharedInstance;
5243

44+
/** Returns a set of events that have been prioritized for the given target.
45+
*
46+
* @param target The target to check. CCT, FLL, and CSH are currently supported by this class.
47+
* @return The set of events prioritized so far.
48+
*/
49+
- (nullable NSSet *)eventsForTarget:(GDTCORTarget)target;
50+
5351
NS_ASSUME_NONNULL_END
5452

5553
@end

GoogleDataTransportCCTSupport/GDTCCTTests/Unit/GDTCCTPrioritizerTest.m

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -56,11 +56,9 @@ - (void)testPrioritizeEvent {
5656
[prioritizer prioritizeEvent:[_CCTGenerator generateEvent:GDTCOREventQosDefault]];
5757
[prioritizer prioritizeEvent:[_FLLGenerator generateEvent:GDTCOREventQosDefault]];
5858
[prioritizer prioritizeEvent:[_CSHGenerator generateEvent:GDTCOREventQosDefault]];
59-
dispatch_sync(prioritizer.queue, ^{
60-
XCTAssertEqual(prioritizer.CCTEvents.count, 1);
61-
XCTAssertEqual(prioritizer.FLLEvents.count, 1);
62-
XCTAssertEqual(prioritizer.CSHEvents.count, 1);
63-
});
59+
XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCCT].count, 1);
60+
XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetFLL].count, 1);
61+
XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCSH].count, 1);
6462
}
6563

6664
/** Tests prioritizing multiple events. */
@@ -75,11 +73,9 @@ - (void)testPrioritizeMultipleEvents {
7573
[prioritizer prioritizeEvent:[_CCTGenerator generateEvent:GDTCOREventQosDefault]];
7674
[prioritizer prioritizeEvent:[_CSHGenerator generateEvent:GDTCOREventQosDefault]];
7775
[prioritizer prioritizeEvent:[_CCTGenerator generateEvent:GDTCOREventQosDefault]];
78-
dispatch_sync(prioritizer.queue, ^{
79-
XCTAssertEqual(prioritizer.CCTEvents.count, 5);
80-
XCTAssertEqual(prioritizer.FLLEvents.count, 2);
81-
XCTAssertEqual(prioritizer.CSHEvents.count, 2);
82-
});
76+
XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCCT].count, 5);
77+
XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetFLL].count, 2);
78+
XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCSH].count, 2);
8379
}
8480

8581
/** Tests unprioritizing events. */
@@ -94,11 +90,9 @@ - (void)testPackageDelivered {
9490
[prioritizer prioritizeEvent:[_CCTGenerator generateEvent:GDTCOREventQosDefault]];
9591
[prioritizer prioritizeEvent:[_CSHGenerator generateEvent:GDTCOREventQosDefault]];
9692
[prioritizer prioritizeEvent:[_CCTGenerator generateEvent:GDTCOREventQosDefault]];
97-
dispatch_sync(prioritizer.queue, ^{
98-
XCTAssertEqual(prioritizer.CCTEvents.count, 5);
99-
XCTAssertEqual(prioritizer.FLLEvents.count, 2);
100-
XCTAssertEqual(prioritizer.CSHEvents.count, 2);
101-
});
93+
XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCCT].count, 5);
94+
XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetFLL].count, 2);
95+
XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCSH].count, 2);
10296
GDTCORUploadPackage *package =
10397
[prioritizer uploadPackageWithTarget:kGDTCORTargetFLL
10498
conditions:GDTCORUploadConditionWifiData];
@@ -191,7 +185,7 @@ - (void)testEncodingAndDecoding {
191185
NSError *error;
192186
dispatch_sync(prioritizer.queue, ^{
193187
});
194-
XCTAssertEqual(prioritizer.CCTEvents.count, 1);
188+
XCTAssertEqual([prioritizer eventsForTarget:kGDTCORTargetCCT].count, 1);
195189
NSData *prioritizerData = GDTCOREncodeArchive(prioritizer, nil, &error);
196190
XCTAssertNil(error);
197191
XCTAssertNotNil(prioritizerData);
@@ -202,7 +196,8 @@ - (void)testEncodingAndDecoding {
202196
XCTAssertNil(error);
203197
XCTAssertNotNil(unarchivedPrioritizer);
204198
XCTAssertEqual([prioritizer hash], [prioritizer hash]);
205-
XCTAssertEqualObjects(prioritizer.CCTEvents, unarchivedPrioritizer.CCTEvents);
199+
XCTAssertEqualObjects([prioritizer eventsForTarget:kGDTCORTargetCCT],
200+
[unarchivedPrioritizer eventsForTarget:kGDTCORTargetCCT]);
206201
}
207202

208203
@end

0 commit comments

Comments
 (0)